Skip to content
Permalink
Browse files Browse the repository at this point in the history
[Security/Http] Remove CSRF tokens from storage on successful login
  • Loading branch information
nicolas-grekas committed Jan 24, 2023
1 parent fa1827c commit 5909d74
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 7 deletions.
Expand Up @@ -63,6 +63,7 @@

<service id="security.authentication.session_strategy" class="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy">
<argument>%security.authentication.session_strategy.strategy%</argument>
<argument type="service" id="security.csrf.token_storage" on-invalid="ignore" />
</service>
<service id="Symfony\Component\Security\Http\Session\SessionAuthenticationStrategyInterface" alias="security.authentication.session_strategy" />

Expand Down
Expand Up @@ -19,12 +19,15 @@ class CsrfFormLoginTest extends AbstractWebTestCase
public function testFormLoginAndLogoutWithCsrfTokens($config)
{
$client = $this->createClient(['test_case' => 'CsrfFormLogin', 'root_config' => $config]);
static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');

$form = $client->request('GET', '/login')->selectButton('login')->form();
$form['user_login[username]'] = 'johannes';
$form['user_login[password]'] = 'test';
$client->submit($form);

$this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo'));

$this->assertRedirect($client->getResponse(), '/profile');

$crawler = $client->followRedirect();
Expand All @@ -48,11 +51,14 @@ public function testFormLoginAndLogoutWithCsrfTokens($config)
public function testFormLoginWithInvalidCsrfToken($config)
{
$client = $this->createClient(['test_case' => 'CsrfFormLogin', 'root_config' => $config]);
static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');

$form = $client->request('GET', '/login')->selectButton('login')->form();
$form['user_login[_token]'] = '';
$client->submit($form);

$this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));

$this->assertRedirect($client->getResponse(), '/login');

$text = $client->followRedirect()->text(null, true);
Expand Down
Expand Up @@ -36,15 +36,13 @@ public function testSessionLessRememberMeLogout()
public function testCsrfTokensAreClearedOnLogout()
{
$client = $this->createClient(['test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml']);
static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');

$client->request('POST', '/login', [
'_username' => 'johannes',
'_password' => 'test',
]);

$this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo'));
static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');

$client->request('GET', '/logout');

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/SecurityBundle/composer.json
Expand Up @@ -25,7 +25,7 @@
"symfony/security-core": "^4.4",
"symfony/security-csrf": "^4.2|^5.0",
"symfony/security-guard": "^4.2|^5.0",
"symfony/security-http": "^4.4.5"
"symfony/security-http": "^4.4.50"
},
"require-dev": {
"doctrine/annotations": "^1.10.4",
Expand Down
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;

/**
* The default session strategy implementation.
Expand All @@ -31,10 +32,15 @@ class SessionAuthenticationStrategy implements SessionAuthenticationStrategyInte
public const INVALIDATE = 'invalidate';

private $strategy;
private $csrfTokenStorage = null;

public function __construct(string $strategy)
public function __construct(string $strategy, ClearableTokenStorageInterface $csrfTokenStorage = null)
{
$this->strategy = $strategy;

if (self::MIGRATE === $strategy) {
$this->csrfTokenStorage = $csrfTokenStorage;
}
}

/**
Expand All @@ -47,10 +53,12 @@ public function onAuthentication(Request $request, TokenInterface $token)
return;

case self::MIGRATE:
// Note: this logic is duplicated in several authentication listeners
// until Symfony 5.0 due to a security fix with BC compat
$request->getSession()->migrate(true);

if ($this->csrfTokenStorage) {
$this->csrfTokenStorage->clear();
}

return;

case self::INVALIDATE:
Expand Down
Expand Up @@ -14,6 +14,7 @@
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Session\SessionInterface;
use Symfony\Component\Security\Csrf\TokenStorage\ClearableTokenStorageInterface;
use Symfony\Component\Security\Http\Session\SessionAuthenticationStrategy;
use Symfony\Component\Security\Http\Tests\Fixtures\TokenInterface;

Expand Down Expand Up @@ -57,6 +58,18 @@ public function testSessionIsInvalidated()
$strategy->onAuthentication($this->getRequest($session), $this->createMock(TokenInterface::class));
}

public function testCsrfTokensAreCleared()
{
$session = $this->createMock(SessionInterface::class);
$session->expects($this->once())->method('migrate')->with($this->equalTo(true));

$csrfStorage = $this->createMock(ClearableTokenStorageInterface::class);
$csrfStorage->expects($this->once())->method('clear');

$strategy = new SessionAuthenticationStrategy(SessionAuthenticationStrategy::MIGRATE, $csrfStorage);
$strategy->onAuthentication($this->getRequest($session), $this->createMock(TokenInterface::class));
}

private function getRequest($session = null)
{
$request = $this->createMock(Request::class);
Expand Down

0 comments on commit 5909d74

Please sign in to comment.