Skip to content

Commit

Permalink
feature #37359 [Security] Add event to inspect authenticated token be…
Browse files Browse the repository at this point in the history
…fore it becomes effective (scheb)

This PR was squashed before being merged into the 5.2-dev branch.

Discussion
----------

[Security] Add event to inspect authenticated token before it becomes effective

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| License       | MIT
| Doc PR        | n/a

Hello there, I'm the author of `scheb/two-factor-bundle`, which extends Symfony's security layer with two-factor authentication. I've been closely following the recent changes by @wouterj to rework the security layer with "authenticators" (great work!). While I managed to make my bundle work with authenticators, I see some limitations in the security layer that I'd like to address to make such extensions easier to implement.

This PR adds a new event, which is disapatched right after the authenticated token has been created by the authenticator, to "announce" it to the application *before* it becomes effective to the security system. The event works similar to `ResponseEvent`, but for security token. It allows listeners to inspect the new token before it becomes effective and - most importantly - apply modifications to it. So components other than the authenticator will be able to influence how the security token looks like, that will be set to the security layer on successful authentication.

Why would you want to do this? Of course I'm looking at this from the 2fa perspective. To make 2fa work, it's necessary to prevent a newly created authenticated token from becoming visible to the security system and therefore exposing its privileges/roles. This is done by replacing the authenticated token with a temporary "TwoFactorToken". Currently I'm doing this through dependency injection, getting all the registered authenticators and decorating them with my own token-exchange logic. This is not very clean and overly complicated, but it works. Adding this event as a hook-in point would allow for a much cleaner integration for any component that wants to have a saying in how the security token should look like.

Commits
-------

2030964 [Security] Add event to inspect authenticated token before it becomes effective
  • Loading branch information
fabpot committed Aug 12, 2020
2 parents 3cd1123 + 2030964 commit 31c194f
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 0 deletions.
Expand Up @@ -26,6 +26,7 @@
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\AuthenticationTokenCreatedEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\Event\LoginFailureEvent;
Expand Down Expand Up @@ -70,6 +71,9 @@ public function authenticateUser(UserInterface $user, AuthenticatorInterface $au
// create an authenticated token for the User
$token = $authenticator->createAuthenticatedToken($passport = new SelfValidatingPassport($user, $badges), $this->firewallName);

// announce the authenticated token
$token = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($token))->getAuthenticatedToken();

// authenticate this in the system
return $this->handleAuthenticationSuccess($token, $passport, $request, $authenticator);
}
Expand Down Expand Up @@ -167,6 +171,10 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req

// create the authenticated token
$authenticatedToken = $authenticator->createAuthenticatedToken($passport, $this->firewallName);

// announce the authenticated token
$authenticatedToken = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($authenticatedToken))->getAuthenticatedToken();

if (true === $this->eraseCredentials) {
$authenticatedToken->eraseCredentials();
}
Expand Down
@@ -0,0 +1,40 @@
<?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\Contracts\EventDispatcher\Event;

/**
* When a newly authenticated security token was created, before it becomes effective in the security system.
*
* @author Christian Scheb <me@christianscheb.de>
*/
class AuthenticationTokenCreatedEvent extends Event
{
private $authenticatedToken;

public function __construct(TokenInterface $token)
{
$this->authenticatedToken = $token;
}

public function getAuthenticatedToken(): TokenInterface
{
return $this->authenticatedToken;
}

public function setAuthenticatedToken(TokenInterface $authenticatedToken): void
{
$this->authenticatedToken = $authenticatedToken;
}
}
Expand Up @@ -24,6 +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\AuthenticationTokenCreatedEvent;
use Symfony\Component\Security\Http\Event\CheckPassportEvent;

class AuthenticatorManagerTest extends TestCase
Expand Down Expand Up @@ -154,6 +155,29 @@ public function provideEraseCredentialsData()
yield [false];
}

public function testAuthenticateRequestCanModifyTokenFromEvent(): void
{
$authenticator = $this->createAuthenticator();
$this->request->attributes->set('_security_authenticators', [$authenticator]);

$authenticator->expects($this->any())->method('authenticate')->willReturn(new SelfValidatingPassport($this->user));

$authenticator->expects($this->any())->method('createAuthenticatedToken')->willReturn($this->token);

$modifiedToken = $this->createMock(TokenInterface::class);
$listenerCalled = false;
$this->eventDispatcher->addListener(AuthenticationTokenCreatedEvent::class, function (AuthenticationTokenCreatedEvent $event) use (&$listenerCalled, $modifiedToken) {
$event->setAuthenticatedToken($modifiedToken);
$listenerCalled = true;
});

$this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken));

$manager = $this->createManager([$authenticator]);
$this->assertNull($manager->authenticateRequest($this->request));
$this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called');
}

public function testAuthenticateUser()
{
$authenticator = $this->createAuthenticator();
Expand All @@ -166,6 +190,26 @@ public function testAuthenticateUser()
$manager->authenticateUser($this->user, $authenticator, $this->request);
}

public function testAuthenticateUserCanModifyTokenFromEvent(): void
{
$authenticator = $this->createAuthenticator();
$authenticator->expects($this->any())->method('createAuthenticatedToken')->willReturn($this->token);
$authenticator->expects($this->any())->method('onAuthenticationSuccess')->willReturn($this->response);

$modifiedToken = $this->createMock(TokenInterface::class);
$listenerCalled = false;
$this->eventDispatcher->addListener(AuthenticationTokenCreatedEvent::class, function (AuthenticationTokenCreatedEvent $event) use (&$listenerCalled, $modifiedToken) {
$event->setAuthenticatedToken($modifiedToken);
$listenerCalled = true;
});

$this->tokenStorage->expects($this->once())->method('setToken')->with($this->identicalTo($modifiedToken));

$manager = $this->createManager([$authenticator]);
$manager->authenticateUser($this->user, $authenticator, $this->request);
$this->assertTrue($listenerCalled, 'The AuthenticationTokenCreatedEvent listener is not called');
}

public function testInteractiveAuthenticator()
{
$authenticator = $this->createMock(InteractiveAuthenticatorInterface::class);
Expand Down

0 comments on commit 31c194f

Please sign in to comment.