Skip to content

Commit

Permalink
Merge branch '4.4' into 5.4
Browse files Browse the repository at this point in the history
* 4.4:
  [Security/Http] Remove CSRF tokens from storage on successful login
  [HttpKernel] Remove private headers before storing responses with HttpCache
  • Loading branch information
nicolas-grekas committed Jan 30, 2023
2 parents e16ac30 + 076fd20 commit 1a049b7
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 12 deletions.
5 changes: 4 additions & 1 deletion Resources/config/security.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@
->set('security.authentication.trust_resolver', AuthenticationTrustResolver::class)

->set('security.authentication.session_strategy', SessionAuthenticationStrategy::class)
->args([param('security.authentication.session_strategy.strategy')])
->args([
param('security.authentication.session_strategy.strategy'),
service('security.csrf.token_storage')->ignoreOnInvalid(),
])
->alias(SessionAuthenticationStrategyInterface::class, 'security.authentication.session_strategy')

->set('security.authentication.session_strategy_noop', SessionAuthenticationStrategy::class)
Expand Down
40 changes: 40 additions & 0 deletions Tests/Functional/CsrfFormLoginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@

namespace Symfony\Bundle\SecurityBundle\Tests\Functional;

use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Event\RequestEvent;
use Symfony\Component\HttpKernel\KernelEvents;

class CsrfFormLoginTest extends AbstractWebTestCase
{
/**
Expand All @@ -20,6 +26,10 @@ public function testFormLoginAndLogoutWithCsrfTokens($options)
{
$client = $this->createClient($options);

$this->callInRequestContext($client, function () {
static::getContainer()->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';
Expand All @@ -40,6 +50,10 @@ public function testFormLoginAndLogoutWithCsrfTokens($options)
$client->click($logoutLinks[0]);

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

$this->callInRequestContext($client, function () {
$this->assertFalse(static::getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
});
}

/**
Expand All @@ -49,6 +63,10 @@ public function testFormLoginWithInvalidCsrfToken($options)
{
$client = $this->createClient($options);

$this->callInRequestContext($client, function () {
static::getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
});

$form = $client->request('GET', '/login')->selectButton('login')->form();
$form['user_login[_token]'] = '';
$client->submit($form);
Expand All @@ -57,6 +75,10 @@ public function testFormLoginWithInvalidCsrfToken($options)

$text = $client->followRedirect()->text(null, true);
$this->assertStringContainsString('Invalid CSRF token.', $text);

$this->callInRequestContext($client, function () {
$this->assertTrue(static::getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
});
}

/**
Expand Down Expand Up @@ -202,4 +224,22 @@ public function provideLegacyClientOptions()
yield [['test_case' => 'CsrfFormLogin', 'root_config' => 'legacy_config.yml', 'enable_authenticator_manager' => false]];
yield [['test_case' => 'CsrfFormLogin', 'root_config' => 'legacy_routes_as_path.yml', 'enable_authenticator_manager' => false]];
}

private function callInRequestContext(KernelBrowser $client, callable $callable): void
{
/** @var EventDispatcherInterface $eventDispatcher */
$eventDispatcher = static::getContainer()->get(EventDispatcherInterface::class);
$wrappedCallable = function (RequestEvent $event) use (&$callable) {
$callable();
$event->setResponse(new Response(''));
$event->stopPropagation();
};

$eventDispatcher->addListener(KernelEvents::REQUEST, $wrappedCallable);
try {
$client->request('GET', '/'.uniqid('', true));
} finally {
$eventDispatcher->removeListener(KernelEvents::REQUEST, $wrappedCallable);
}
}
}
12 changes: 2 additions & 10 deletions Tests/Functional/LogoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,14 @@ public function testCsrfTokensAreClearedOnLogout()
{
$client = $this->createClient(['enable_authenticator_manager' => true, 'test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml']);
$client->disableReboot();
$this->callInRequestContext($client, function () {
static::getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
});

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

$this->callInRequestContext($client, function () {
$this->assertTrue(static::getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', static::getContainer()->get('security.csrf.token_storage')->getToken('foo'));
static::getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
});

$client->request('GET', '/logout');
Expand All @@ -52,18 +48,14 @@ public function testLegacyCsrfTokensAreClearedOnLogout()
{
$client = $this->createClient(['enable_authenticator_manager' => false, 'test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml']);
$client->disableReboot();
$this->callInRequestContext($client, function () {
static::getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
});

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

$this->callInRequestContext($client, function () {
$this->assertTrue(static::getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', static::getContainer()->get('security.csrf.token_storage')->getToken('foo'));
static::getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
});

$client->request('GET', '/logout');
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"symfony/security-core": "^5.4|^6.0",
"symfony/security-csrf": "^4.4|^5.0|^6.0",
"symfony/security-guard": "^5.3",
"symfony/security-http": "^5.4|^6.0"
"symfony/security-http": "^5.4.20|~6.0.20|~6.1.12|^6.2.6"
},
"require-dev": {
"doctrine/annotations": "^1.10.4|^2",
Expand Down

0 comments on commit 1a049b7

Please sign in to comment.