Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Avoid migration on stateless firewalls #27452

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 10, 2018
Merged

Avoid migration on stateless firewalls #27452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions 1 ...Bundle/SecurityBundle/DependencyInjection/Security/Factory/GuardAuthenticationFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.guard'));
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, $authenticatorReferences);
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

// determine the entryPointId to use
$entryPointId = $this->determineEntryPoint($defaultEntryPoint, $config);
Expand Down
1 change: 1 addition & 0 deletions 1 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpBasicFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.basic'));
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, new Reference($entryPointId));
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

return array($provider, $listenerId, $entryPointId);
}
Expand Down
1 change: 1 addition & 0 deletions 1 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/HttpDigestFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener->replaceArgument(1, new Reference($userProvider));
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, new Reference($entryPointId));
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

return array($provider, $listenerId, $entryPointId);
}
Expand Down
1 change: 1 addition & 0 deletions 1 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/RemoteUserFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.remote_user'));
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, $config['user']);
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

return array($providerId, $listenerId, $defaultEntryPoint);
}
Expand Down
1 change: 1 addition & 0 deletions 1 ...le/SecurityBundle/DependencyInjection/Security/Factory/SimplePreAuthenticationFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener = $container->setDefinition($listenerId, new DefinitionDecorator('security.authentication.listener.simple_preauth'));
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, new Reference($config['authenticator']));
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

return array($provider, $listenerId, null);
}
Expand Down
1 change: 1 addition & 0 deletions 1 src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/X509Factory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public function create(ContainerBuilder $container, $id, $config, $userProvider,
$listener->replaceArgument(2, $id);
$listener->replaceArgument(3, $config['user']);
$listener->replaceArgument(4, $config['credentials']);
$listener->addMethodCall('setSessionAuthenticationStrategy', array(new Reference('security.authentication.session_strategy.'.$id)));

return array($providerId, $listenerId, $defaultEntryPoint);
}
Expand Down
4 changes: 4 additions & 0 deletions 4 src/Symfony/Bundle/SecurityBundle/DependencyInjection/SecurityExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,11 @@ private function createFirewall(ContainerBuilder $container, $id, $firewall, &$a
}

$listeners[] = new Reference($this->createContextListener($container, $contextKey));
$sessionStrategyId = 'security.authentication.session_strategy';
} else {
$sessionStrategyId = 'security.authentication.session_strategy_noop';
}
$container->setAlias(new Alias('security.authentication.session_strategy.'.$id, false), $sessionStrategyId);

// Logout listener
$logoutListenerId = null;
Expand Down
4 changes: 4 additions & 0 deletions 4 src/Symfony/Bundle/SecurityBundle/Resources/config/security.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@
<argument>%security.authentication.session_strategy.strategy%</argument>
</service>

<service id="security.authentication.session_strategy_noop" class="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy" public="false">
<argument>none</argument>
</service>

<service id="security.encoder_factory.generic" class="%security.encoder_factory.generic.class%" public="false">
<argument type="collection" />
</service>
Expand Down
2 changes: 1 addition & 1 deletion 2 src/Symfony/Bundle/SecurityBundle/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"require": {
"php": ">=5.3.9",
"ext-xml": "*",
"symfony/security": "^2.8.41|^3.4.11",
"symfony/security": "^2.8.42|^3.4.12",
Copy link
Member Author

Choose a reason for hiding this comment

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

I bumped both versions - pretty sure that's correct :)

"symfony/security-acl": "~2.7|~3.0.0",
"symfony/http-kernel": "~2.7|~3.0.0",
"symfony/polyfill-php70": "~1.0"
Expand Down
24 changes: 16 additions & 8 deletions 24 src/Symfony/Component/Security/Guard/GuardAuthenticatorHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Symfony\Component\Security\Core\User\UserInterface;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\SecurityEvents;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;

/**
* A utility class that does much of the *work* during the guard authentication process.
Expand All @@ -32,8 +33,8 @@
class GuardAuthenticatorHandler
{
private $tokenStorage;

private $dispatcher;
private $sessionStrategy;

public function __construct(TokenStorageInterface $tokenStorage, EventDispatcherInterface $eventDispatcher = null)
{
Expand All @@ -46,7 +47,7 @@ public function __construct(TokenStorageInterface $tokenStorage, EventDispatcher
*/
public function authenticateWithToken(TokenInterface $token, Request $request)
{
$this->migrateSession($request);
$this->migrateSession($request, $token);
$this->tokenStorage->setToken($token);

if (null !== $this->dispatcher) {
Expand Down Expand Up @@ -129,15 +130,22 @@ public function handleAuthenticationFailure(AuthenticationException $authenticat
));
}

private function migrateSession(Request $request)
/**
* Call this method if your authentication token is stored to a session.
*
* @final since version 2.8
Copy link
Member

Choose a reason for hiding this comment

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

"since version 2.8" should be removed. I don't think we are doing that anywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

We do when we want to hint that the @final has been added after the method has been introduced. i.e. when it's deprecated to extend from it.
Which is not the case here, so yes, this has to be removed.

Copy link
Member

Choose a reason for hiding this comment

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

see 5c2b2bb

*/
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
{
$this->sessionStrategy = $sessionStrategy;
}

private function migrateSession(Request $request, TokenInterface $token)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
$this->sessionStrategy->onAuthentication($request, $token);
}
}
37 changes: 37 additions & 0 deletions 37 src/Symfony/Component/Security/Guard/Tests/GuardAuthenticatorHandlerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class GuardAuthenticatorHandlerTest extends TestCase
private $dispatcher;
private $token;
private $request;
private $sessionStrategy;
private $guardAuthenticator;

public function testAuthenticateWithToken()
Expand Down Expand Up @@ -117,12 +118,38 @@ public function getTokenClearingTests()
return $tests;
}

public function testNoFailureIfSessionStrategyNotPassed()
{
$this->configurePreviousSession();

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

$handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher);
$handler->authenticateWithToken($this->token, $this->request);
}

public function testSessionStrategyIsCalled()
{
$this->configurePreviousSession();

$this->sessionStrategy->expects($this->once())
->method('onAuthentication')
->with($this->request, $this->token);

$handler = new GuardAuthenticatorHandler($this->tokenStorage, $this->dispatcher);
$handler->setSessionAuthenticationStrategy($this->sessionStrategy);
$handler->authenticateWithToken($this->token, $this->request);
}

protected function setUp()
{
$this->tokenStorage = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface')->getMock();
$this->dispatcher = $this->getMockBuilder('Symfony\Component\EventDispatcher\EventDispatcherInterface')->getMock();
$this->token = $this->getMockBuilder('Symfony\Component\Security\Core\Authentication\Token\TokenInterface')->getMock();
$this->request = new Request(array(), array(), array(), array(), array(), array());
$this->sessionStrategy = $this->getMockBuilder('Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface')->getMock();
$this->guardAuthenticator = $this->getMockBuilder('Symfony\Component\Security\Guard\GuardAuthenticatorInterface')->getMock();
}

Expand All @@ -134,4 +161,14 @@ protected function tearDown()
$this->request = null;
$this->guardAuthenticator = null;
}

private function configurePreviousSession()
{
$session = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\SessionInterface')->getMock();
$session->expects($this->any())
->method('getName')
->willReturn('test_session_name');
$this->request->setSession($session);
$this->request->cookies->set('test_session_name', 'session_cookie_val');
}
}
24 changes: 17 additions & 7 deletions 24 src/Symfony/Component/Security/Http/Firewall/AbstractPreAuthenticatedListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\PreAuthenticatedToken;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Event\InteractiveLoginEvent;
use Symfony\Component\Security\Http\SecurityEvents;
Expand All @@ -22,6 +23,7 @@
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;

/**
* AbstractPreAuthenticatedListener is the base class for all listener that
Expand All @@ -37,6 +39,7 @@ abstract class AbstractPreAuthenticatedListener implements ListenerInterface
private $authenticationManager;
private $providerKey;
private $dispatcher;
private $sessionStrategy;

public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, $providerKey, LoggerInterface $logger = null, EventDispatcherInterface $dispatcher = null)
{
Expand Down Expand Up @@ -83,7 +86,7 @@ final public function handle(GetResponseEvent $event)
$this->logger->info('Pre-authentication successful.', array('token' => (string) $token));
}

$this->migrateSession($request);
$this->migrateSession($request, $token);

$this->tokenStorage->setToken($token);

Expand All @@ -96,6 +99,16 @@ final public function handle(GetResponseEvent $event)
}
}

/**
* Call this method if your authentication token is stored to a session.
*
* @final since version 2.8
*/
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
{
$this->sessionStrategy = $sessionStrategy;
}

/**
* Clears a PreAuthenticatedToken for this provider (if present).
*/
Expand All @@ -118,15 +131,12 @@ private function clearToken(AuthenticationException $exception)
*/
abstract protected function getPreAuthenticatedData(Request $request);

private function migrateSession(Request $request)
private function migrateSession(Request $request, TokenInterface $token)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
$this->sessionStrategy->onAuthentication($request, $token);
}
}
24 changes: 17 additions & 7 deletions 24 src/Symfony/Component/Security/Http/Firewall/BasicAuthenticationListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\AuthenticationManagerInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;

/**
* BasicAuthenticationListener implements Basic HTTP authentication.
Expand All @@ -33,6 +35,7 @@ class BasicAuthenticationListener implements ListenerInterface
private $authenticationEntryPoint;
private $logger;
private $ignoreFailure;
private $sessionStrategy;

public function __construct(TokenStorageInterface $tokenStorage, AuthenticationManagerInterface $authenticationManager, $providerKey, AuthenticationEntryPointInterface $authenticationEntryPoint, LoggerInterface $logger = null)
{
Expand Down Expand Up @@ -72,7 +75,7 @@ public function handle(GetResponseEvent $event)
try {
$token = $this->authenticationManager->authenticate(new UsernamePasswordToken($username, $request->headers->get('PHP_AUTH_PW'), $this->providerKey));

$this->migrateSession($request);
$this->migrateSession($request, $token);

$this->tokenStorage->setToken($token);
} catch (AuthenticationException $e) {
Expand All @@ -93,15 +96,22 @@ public function handle(GetResponseEvent $event)
}
}

private function migrateSession(Request $request)
/**
* Call this method if your authentication token is stored to a session.
*
* @final since version 2.8
*/
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
{
$this->sessionStrategy = $sessionStrategy;
}

private function migrateSession(Request $request, TokenInterface $token)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
$this->sessionStrategy->onAuthentication($request, $token);
}
}
27 changes: 19 additions & 8 deletions 27 src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Security\Http\Firewall;

use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\User\UserProviderInterface;
use Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint;
use Psr\Log\LoggerInterface;
Expand All @@ -23,6 +24,7 @@
use Symfony\Component\Security\Core\Exception\NonceExpiredException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface;

/**
* DigestAuthenticationListener implements Digest HTTP authentication.
Expand All @@ -36,6 +38,7 @@ class DigestAuthenticationListener implements ListenerInterface
private $providerKey;
private $authenticationEntryPoint;
private $logger;
private $sessionStrategy;

public function __construct(TokenStorageInterface $tokenStorage, UserProviderInterface $provider, $providerKey, DigestAuthenticationEntryPoint $authenticationEntryPoint, LoggerInterface $logger = null)
{
Expand Down Expand Up @@ -117,9 +120,20 @@ public function handle(GetResponseEvent $event)
$this->logger->info('Digest authentication successful.', array('username' => $digestAuth->getUsername(), 'received' => $digestAuth->getResponse()));
}

$this->migrateSession($request);
$token = new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey);
$this->migrateSession($request, $token);

$this->tokenStorage->setToken(new UsernamePasswordToken($user, $user->getPassword(), $this->providerKey));
$this->tokenStorage->setToken($token);
}

/**
* Call this method if your authentication token is stored to a session.
*
* @final since version 2.8
*/
public function setSessionAuthenticationStrategy(SessionAuthenticationStrategyInterface $sessionStrategy)
{
$this->sessionStrategy = $sessionStrategy;
}

private function fail(GetResponseEvent $event, Request $request, AuthenticationException $authException)
Expand All @@ -136,16 +150,13 @@ private function fail(GetResponseEvent $event, Request $request, AuthenticationE
$event->setResponse($this->authenticationEntryPoint->start($request, $authException));
}

private function migrateSession(Request $request)
private function migrateSession(Request $request, TokenInterface $token)
{
if (!$request->hasSession() || !$request->hasPreviousSession()) {
if (!$this->sessionStrategy || !$request->hasSession() || !$request->hasPreviousSession()) {
return;
}

// Destroying the old session is broken in php 5.4.0 - 5.4.10
// See https://bugs.php.net/63379
$destroy = \PHP_VERSION_ID < 50400 || \PHP_VERSION_ID >= 50411;
$request->getSession()->migrate($destroy);
$this->sessionStrategy->onAuthentication($request, $token);
}
}

Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.