diff --git a/CHANGELOG.md b/CHANGELOG.md index 22652b0..a347990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ CHANGELOG ========= +7.2 +--- + + * Add `SameOriginCsrfTokenManager` + +6.0 +--- + + * Remove the `SessionInterface $session` constructor argument of `SessionTokenStorage`, inject a `\Symfony\Component\HttpFoundation\RequestStack $requestStack` instead + * Using `SessionTokenStorage` outside a request context throws a `SessionNotFoundException` + 5.3 --- diff --git a/CsrfToken.php b/CsrfToken.php index 861a510..f3a28d9 100644 --- a/CsrfToken.php +++ b/CsrfToken.php @@ -18,41 +18,35 @@ */ class CsrfToken { - private $id; - private $value; + private string $value; - public function __construct(string $id, ?string $value) - { - $this->id = $id; + public function __construct( + private string $id, + #[\SensitiveParameter] ?string $value, + ) { $this->value = $value ?? ''; } /** * Returns the ID of the CSRF token. - * - * @return string */ - public function getId() + public function getId(): string { return $this->id; } /** * Returns the value of the CSRF token. - * - * @return string */ - public function getValue() + public function getValue(): string { return $this->value; } /** * Returns the value of the CSRF token. - * - * @return string */ - public function __toString() + public function __toString(): string { return $this->value; } diff --git a/CsrfTokenManager.php b/CsrfTokenManager.php index 642b970..1a873ae 100644 --- a/CsrfTokenManager.php +++ b/CsrfTokenManager.php @@ -26,47 +26,44 @@ */ class CsrfTokenManager implements CsrfTokenManagerInterface { - private $generator; - private $storage; - private $namespace; + private TokenGeneratorInterface $generator; + private TokenStorageInterface $storage; + private \Closure|string $namespace; /** - * @param string|RequestStack|callable|null $namespace - * * null: generates a namespace using $_SERVER['HTTPS'] - * * string: uses the given string - * * RequestStack: generates a namespace using the current main request - * * callable: uses the result of this callable (must return a string) + * @param $namespace + * * null: generates a namespace using $_SERVER['HTTPS'] + * * string: uses the given string + * * RequestStack: generates a namespace using the current main request + * * callable: uses the result of this callable (must return a string) */ - public function __construct(?TokenGeneratorInterface $generator = null, ?TokenStorageInterface $storage = null, $namespace = null) + public function __construct(?TokenGeneratorInterface $generator = null, ?TokenStorageInterface $storage = null, string|RequestStack|callable|null $namespace = null) { $this->generator = $generator ?? new UriSafeTokenGenerator(); $this->storage = $storage ?? new NativeSessionTokenStorage(); - $superGlobalNamespaceGenerator = function () { - return !empty($_SERVER['HTTPS']) && 'off' !== strtolower($_SERVER['HTTPS']) ? 'https-' : ''; - }; + $superGlobalNamespaceGenerator = fn () => !empty($_SERVER['HTTPS']) && 'off' !== strtolower($_SERVER['HTTPS']) ? 'https-' : ''; if (null === $namespace) { $this->namespace = $superGlobalNamespaceGenerator; } elseif ($namespace instanceof RequestStack) { - $this->namespace = function () use ($namespace, $superGlobalNamespaceGenerator) { + $this->namespace = static function () use ($namespace, $superGlobalNamespaceGenerator) { if ($request = $namespace->getMainRequest()) { return $request->isSecure() ? 'https-' : ''; } return $superGlobalNamespaceGenerator(); }; - } elseif (\is_callable($namespace) || \is_string($namespace)) { + } elseif ($namespace instanceof \Closure || \is_string($namespace)) { $this->namespace = $namespace; + } elseif (\is_callable($namespace)) { + $this->namespace = $namespace(...); } else { - throw new InvalidArgumentException(sprintf('$namespace must be a string, a callable returning a string, null or an instance of "RequestStack". "%s" given.', get_debug_type($namespace))); + throw new InvalidArgumentException(\sprintf('$namespace must be a string, a callable returning a string, null or an instance of "RequestStack". "%s" given.', get_debug_type($namespace))); } } - /** - * {@inheritdoc} - */ - public function getToken(string $tokenId) + public function getToken(string $tokenId): CsrfToken { $namespacedId = $this->getNamespace().$tokenId; if ($this->storage->hasToken($namespacedId)) { @@ -80,10 +77,7 @@ public function getToken(string $tokenId) return new CsrfToken($tokenId, $this->randomize($value)); } - /** - * {@inheritdoc} - */ - public function refreshToken(string $tokenId) + public function refreshToken(string $tokenId): CsrfToken { $namespacedId = $this->getNamespace().$tokenId; $value = $this->generator->generateToken(); @@ -93,18 +87,12 @@ public function refreshToken(string $tokenId) return new CsrfToken($tokenId, $this->randomize($value)); } - /** - * {@inheritdoc} - */ - public function removeToken(string $tokenId) + public function removeToken(string $tokenId): ?string { return $this->storage->removeToken($this->getNamespace().$tokenId); } - /** - * {@inheritdoc} - */ - public function isTokenValid(CsrfToken $token) + public function isTokenValid(CsrfToken $token): bool { $namespacedId = $this->getNamespace().$token->getId(); if (!$this->storage->hasToken($namespacedId)) { @@ -124,7 +112,7 @@ private function randomize(string $value): string $key = random_bytes(32); $value = $this->xor($value, $key); - return sprintf('%s.%s.%s', substr(md5($key), 0, 1 + (\ord($key[0]) % 32)), rtrim(strtr(base64_encode($key), '+/', '-_'), '='), rtrim(strtr(base64_encode($value), '+/', '-_'), '=')); + return \sprintf('%s.%s.%s', substr(hash('xxh128', $key), 0, 1 + (\ord($key[0]) % 32)), rtrim(strtr(base64_encode($key), '+/', '-_'), '='), rtrim(strtr(base64_encode($value), '+/', '-_'), '=')); } private function derandomize(string $value): string diff --git a/CsrfTokenManagerInterface.php b/CsrfTokenManagerInterface.php index 8e4d72c..14984a9 100644 --- a/CsrfTokenManagerInterface.php +++ b/CsrfTokenManagerInterface.php @@ -27,10 +27,8 @@ interface CsrfTokenManagerInterface * * @param string $tokenId The token ID. You may choose an arbitrary value * for the ID - * - * @return CsrfToken */ - public function getToken(string $tokenId); + public function getToken(string $tokenId): CsrfToken; /** * Generates a new token value for the given ID. @@ -41,10 +39,8 @@ public function getToken(string $tokenId); * * @param string $tokenId The token ID. You may choose an arbitrary value * for the ID - * - * @return CsrfToken */ - public function refreshToken(string $tokenId); + public function refreshToken(string $tokenId): CsrfToken; /** * Invalidates the CSRF token with the given ID, if one exists. @@ -52,12 +48,10 @@ public function refreshToken(string $tokenId); * @return string|null Returns the removed token value if one existed, NULL * otherwise */ - public function removeToken(string $tokenId); + public function removeToken(string $tokenId): ?string; /** * Returns whether the given CSRF token is valid. - * - * @return bool */ - public function isTokenValid(CsrfToken $token); + public function isTokenValid(CsrfToken $token): bool; } diff --git a/README.md b/README.md index a27d877..79277e6 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ The Security CSRF (cross-site request forgery) component provides a class Sponsor ------- -The Security component for Symfony 5.4/6.0 is [backed][1] by [SymfonyCasts][2]. +The Security component for Symfony 7.1 is [backed][1] by [SymfonyCasts][2]. Learn Symfony faster by watching real projects being built and actively coding along with them. SymfonyCasts bridges that learning gap, bringing you video diff --git a/SameOriginCsrfTokenManager.php b/SameOriginCsrfTokenManager.php new file mode 100644 index 0000000..b1e0730 --- /dev/null +++ b/SameOriginCsrfTokenManager.php @@ -0,0 +1,288 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf; + +use Psr\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpKernel\Event\ResponseEvent; + +/** + * This CSRF token manager uses a combination of cookie and headers to validate non-persistent tokens. + * + * This manager is designed to be stateless and compatible with HTTP-caching. + * + * First, we validate the source of the request using the Origin/Referer headers. This relies + * on the app being able to know its own target origin. Don't miss configuring your reverse proxy to + * send the X-Forwarded-* / Forwarded headers if you're behind one. + * + * Then, we validate the request using a cookie and a CsrfToken. If the cookie is found, it should + * contain the same value as the CsrfToken. A JavaScript snippet on the client side is responsible + * for performing this double-submission. The token value should be regenerated on every request + * using a cryptographically secure random generator. + * + * If either double-submit or Origin/Referer headers are missing, it typically indicates that + * JavaScript is disabled on the client side, or that the JavaScript snippet was not properly + * implemented, or that the Origin/Referer headers were filtered out. + * + * Requests lacking both double-submit and origin information are deemed insecure. + * + * When a session is found, a behavioral check is added to ensure that the validation method does not + * downgrade from double-submit to origin checks. This prevents attackers from exploiting potentially + * less secure validation methods once a more secure method has been confirmed as functional. + * + * On HTTPS connections, the cookie is prefixed with "__Host-" to prevent it from being forged on an + * HTTP channel. On the JS side, the cookie should be set with samesite=strict to strengthen the CSRF + * protection. The cookie is always cleared on the response to prevent any further use of the token. + * + * The $checkHeader argument allows the token to be checked in a header instead of or in addition to a + * cookie. This makes it harder for an attacker to forge a request, though it may also pose challenges + * when setting the header depending on the client-side framework in use. + * + * When a fallback CSRF token manager is provided, only tokens listed in the $tokenIds argument will be + * managed by this manager. All other tokens will be delegated to the fallback manager. + * + * @author Nicolas Grekas + */ +final class SameOriginCsrfTokenManager implements CsrfTokenManagerInterface +{ + public const TOKEN_MIN_LENGTH = 24; + + public const CHECK_NO_HEADER = 0; + public const CHECK_HEADER = 1; + public const CHECK_ONLY_HEADER = 2; + + /** + * @param self::CHECK_* $checkHeader + * @param string[] $tokenIds + */ + public function __construct( + private RequestStack $requestStack, + private ?LoggerInterface $logger = null, + private ?CsrfTokenManagerInterface $fallbackCsrfTokenManager = null, + private array $tokenIds = [], + private int $checkHeader = self::CHECK_NO_HEADER, + private string $cookieName = 'csrf-token', + ) { + if (!$cookieName) { + throw new \InvalidArgumentException('The cookie name cannot be empty.'); + } + + if (!preg_match('/^[-a-zA-Z0-9_]+$/D', $cookieName)) { + throw new \InvalidArgumentException('The cookie name contains invalid characters.'); + } + + $this->tokenIds = array_flip($tokenIds); + } + + public function getToken(string $tokenId): CsrfToken + { + if (!isset($this->tokenIds[$tokenId]) && $this->fallbackCsrfTokenManager) { + return $this->fallbackCsrfTokenManager->getToken($tokenId); + } + + return new CsrfToken($tokenId, $this->cookieName); + } + + public function refreshToken(string $tokenId): CsrfToken + { + if (!isset($this->tokenIds[$tokenId]) && $this->fallbackCsrfTokenManager) { + return $this->fallbackCsrfTokenManager->refreshToken($tokenId); + } + + return new CsrfToken($tokenId, $this->cookieName); + } + + public function removeToken(string $tokenId): ?string + { + if (!isset($this->tokenIds[$tokenId]) && $this->fallbackCsrfTokenManager) { + return $this->fallbackCsrfTokenManager->removeToken($tokenId); + } + + return null; + } + + public function isTokenValid(CsrfToken $token): bool + { + if (!isset($this->tokenIds[$token->getId()]) && $this->fallbackCsrfTokenManager) { + return $this->fallbackCsrfTokenManager->isTokenValid($token); + } + + if (!$request = $this->requestStack->getCurrentRequest()) { + $this->logger?->error('CSRF validation failed: No request found.'); + + return false; + } + + if (\strlen($token->getValue()) < self::TOKEN_MIN_LENGTH && $token->getValue() !== $this->cookieName) { + $this->logger?->warning('Invalid double-submit CSRF token.'); + + return false; + } + + if (false === $isValidOrigin = $this->isValidOrigin($request)) { + $this->logger?->warning('CSRF validation failed: origin info doesn\'t match.'); + + return false; + } + + if (false === $isValidDoubleSubmit = $this->isValidDoubleSubmit($request, $token->getValue())) { + return false; + } + + if (null === $isValidOrigin && null === $isValidDoubleSubmit) { + $this->logger?->warning('CSRF validation failed: double-submit and origin info not found.'); + + return false; + } + + // Opportunistically lookup at the session for a previous CSRF validation strategy + $session = $request->hasPreviousSession() ? $request->getSession() : null; + $usageIndexValue = $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0; + $usageIndexReference = \PHP_INT_MIN; + $previousCsrfProtection = (int) $session?->get($this->cookieName); + $usageIndexReference = $usageIndexValue; + $shift = $request->isMethodSafe() ? 8 : 0; + + if ($previousCsrfProtection) { + if (!$isValidOrigin && (1 & ($previousCsrfProtection >> $shift))) { + $this->logger?->warning('CSRF validation failed: origin info was used in a previous request but is now missing.'); + + return false; + } + + if (!$isValidDoubleSubmit && (2 & ($previousCsrfProtection >> $shift))) { + $this->logger?->warning('CSRF validation failed: double-submit info was used in a previous request but is now missing.'); + + return false; + } + } + + if ($isValidOrigin && $isValidDoubleSubmit) { + $csrfProtection = 3; + $this->logger?->debug('CSRF validation accepted using both origin and double-submit info.'); + } elseif ($isValidOrigin) { + $csrfProtection = 1; + $this->logger?->debug('CSRF validation accepted using origin info.'); + } else { + $csrfProtection = 2; + $this->logger?->debug('CSRF validation accepted using double-submit info.'); + } + + if (1 & $csrfProtection) { + // Persist valid origin for both safe and non-safe requests + $previousCsrfProtection |= 1 & (1 << 8); + } + + $request->attributes->set($this->cookieName, ($csrfProtection << $shift) | $previousCsrfProtection); + + return true; + } + + public function clearCookies(Request $request, Response $response): void + { + if (!$request->attributes->has($this->cookieName)) { + return; + } + + $cookieName = ($request->isSecure() ? '__Host-' : '').$this->cookieName; + + foreach ($request->cookies->all() as $name => $value) { + if ($this->cookieName === $value && str_starts_with($name, $cookieName.'_')) { + $response->headers->clearCookie($name, '/', null, $request->isSecure(), false, 'strict'); + } + } + } + + public function persistStrategy(Request $request): void + { + if (!$request->attributes->has($this->cookieName) + || !$request->hasSession(true) + || !($session = $request->getSession())->isStarted() + ) { + return; + } + + $usageIndexValue = $session instanceof Session ? $usageIndexReference = &$session->getUsageIndex() : 0; + $usageIndexReference = \PHP_INT_MIN; + $session->set($this->cookieName, $request->attributes->get($this->cookieName)); + $usageIndexReference = $usageIndexValue; + } + + public function onKernelResponse(ResponseEvent $event): void + { + if (!$event->isMainRequest()) { + return; + } + + $this->clearCookies($event->getRequest(), $event->getResponse()); + $this->persistStrategy($event->getRequest()); + } + + /** + * @return bool|null Whether the origin is valid, null if missing + */ + private function isValidOrigin(Request $request): ?bool + { + $target = $request->getSchemeAndHttpHost().'/'; + $source = 'null'; + + foreach (['Origin', 'Referer'] as $header) { + if (!$request->headers->has($header)) { + continue; + } + $source = $request->headers->get($header); + + if (str_starts_with($source.'/', $target)) { + return true; + } + } + + return 'null' === $source ? null : false; + } + + /** + * @return bool|null Whether the double-submit is valid, null if missing + */ + private function isValidDoubleSubmit(Request $request, string $token): ?bool + { + if ($this->cookieName === $token) { + return null; + } + + if ($this->checkHeader && $request->headers->get($this->cookieName, $token) !== $token) { + $this->logger?->warning('CSRF validation failed: wrong token found in header info.'); + + return false; + } + + $cookieName = ($request->isSecure() ? '__Host-' : '').$this->cookieName; + + if (self::CHECK_ONLY_HEADER === $this->checkHeader) { + if (!$request->headers->has($this->cookieName)) { + return null; + } + + $request->cookies->set($cookieName.'_'.$token, $this->cookieName); // Ensure clearCookie() can remove any cookie filtered by a reverse-proxy + + return true; + } + + if (($request->cookies->all()[$cookieName.'_'.$token] ?? null) !== $this->cookieName && !($this->checkHeader && $request->headers->has($this->cookieName))) { + return null; + } + + return true; + } +} diff --git a/Tests/CsrfTokenManagerTest.php b/Tests/CsrfTokenManagerTest.php index c1bff05..d33adfb 100644 --- a/Tests/CsrfTokenManagerTest.php +++ b/Tests/CsrfTokenManagerTest.php @@ -519,9 +519,7 @@ private function getClosureMocks(): array { [$generator, $storage] = $this->getGeneratorAndStorage(); - return ['generated-', new CsrfTokenManager($generator, $storage, function () { - return 'generated-'; - }), $storage, $generator]; + return ['generated-', new CsrfTokenManager($generator, $storage, fn () => 'generated-'), $storage, $generator]; } private function getRequestStackWithEmptyNamespaceMocks(): array @@ -548,8 +546,6 @@ protected function setUp(): void protected function tearDown(): void { - parent::tearDown(); - unset($_SERVER['HTTPS']); } } diff --git a/Tests/SameOriginCsrfTokenManagerTest.php b/Tests/SameOriginCsrfTokenManagerTest.php new file mode 100644 index 0000000..0a215d2 --- /dev/null +++ b/Tests/SameOriginCsrfTokenManagerTest.php @@ -0,0 +1,261 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\Security\Csrf\Tests; + +use PHPUnit\Framework\TestCase; +use Psr\Log\LoggerInterface; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\RequestStack; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpFoundation\Session\Session; +use Symfony\Component\HttpKernel\Event\ResponseEvent; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\Component\Security\Csrf\CsrfToken; +use Symfony\Component\Security\Csrf\SameOriginCsrfTokenManager; + +class SameOriginCsrfTokenManagerTest extends TestCase +{ + private $requestStack; + private $logger; + private $csrfTokenManager; + + protected function setUp(): void + { + $this->requestStack = new RequestStack(); + $this->logger = $this->createMock(LoggerInterface::class); + $this->csrfTokenManager = new SameOriginCsrfTokenManager($this->requestStack, $this->logger); + } + + public function testInvalidCookieName() + { + $this->expectException(\InvalidArgumentException::class); + new SameOriginCsrfTokenManager($this->requestStack, $this->logger, null, [], SameOriginCsrfTokenManager::CHECK_NO_HEADER, ''); + } + + public function testInvalidCookieNameCharacters() + { + $this->expectException(\InvalidArgumentException::class); + new SameOriginCsrfTokenManager($this->requestStack, $this->logger, null, [], SameOriginCsrfTokenManager::CHECK_NO_HEADER, 'invalid name!'); + } + + public function testGetToken() + { + $tokenId = 'test_token'; + $token = $this->csrfTokenManager->getToken($tokenId); + + $this->assertInstanceOf(CsrfToken::class, $token); + $this->assertSame($tokenId, $token->getId()); + } + + public function testNoRequest() + { + $token = new CsrfToken('test_token', 'test_value'); + + $this->logger->expects($this->once())->method('error')->with('CSRF validation failed: No request found.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testInvalidTokenLength() + { + $request = new Request(); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', ''); + + $this->logger->expects($this->once())->method('warning')->with('Invalid double-submit CSRF token.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testInvalidOrigin() + { + $request = new Request(); + $request->headers->set('Origin', 'http://malicious.com'); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $this->logger->expects($this->once())->method('warning')->with('CSRF validation failed: origin info doesn\'t match.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testValidOrigin() + { + $request = new Request(); + $request->headers->set('Origin', $request->getSchemeAndHttpHost()); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using origin info.'); + $this->assertTrue($this->csrfTokenManager->isTokenValid($token)); + $this->assertSame(1 << 8, $request->attributes->get('csrf-token')); + } + + public function testValidRefererInvalidOrigin() + { + $request = new Request(); + $request->headers->set('Origin', 'http://localhost:1234'); + $request->headers->set('Referer', $request->getSchemeAndHttpHost()); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using origin info.'); + $this->assertTrue($this->csrfTokenManager->isTokenValid($token)); + $this->assertSame(1 << 8, $request->attributes->get('csrf-token')); + } + + public function testValidOriginAfterDoubleSubmit() + { + $session = $this->createMock(Session::class); + $request = new Request(); + $request->setSession($session); + $request->headers->set('Origin', $request->getSchemeAndHttpHost()); + $request->cookies->set('sess', 'id'); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $session->expects($this->once())->method('getName')->willReturn('sess'); + $session->expects($this->once())->method('get')->with('csrf-token')->willReturn(2 << 8); + $this->logger->expects($this->once())->method('warning')->with('CSRF validation failed: double-submit info was used in a previous request but is now missing.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testMissingPreviousOrigin() + { + $session = $this->createMock(Session::class); + $request = new Request(); + $request->cookies->set('csrf-token_'.str_repeat('a', 24), 'csrf-token'); + $request->setSession($session); + $request->cookies->set('sess', 'id'); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $session->expects($this->once())->method('getName')->willReturn('sess'); + $session->expects($this->once())->method('get')->with('csrf-token')->willReturn(1 << 8); + $this->logger->expects($this->once())->method('warning')->with('CSRF validation failed: origin info was used in a previous request but is now missing.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testValidDoubleSubmit() + { + $request = new Request(); + $request->cookies->set('csrf-token_'.str_repeat('a', 24), 'csrf-token'); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using double-submit info.'); + $this->assertTrue($this->csrfTokenManager->isTokenValid($token)); + $this->assertSame(2 << 8, $request->attributes->get('csrf-token')); + } + + public function testCheckOnlyHeader() + { + $csrfTokenManager = new SameOriginCsrfTokenManager($this->requestStack, $this->logger, null, [], SameOriginCsrfTokenManager::CHECK_ONLY_HEADER); + + $request = new Request(); + $tokenValue = str_repeat('a', 24); + $request->headers->set('csrf-token', $tokenValue); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', $tokenValue); + + $this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using double-submit info.'); + $this->assertTrue($csrfTokenManager->isTokenValid($token)); + $this->assertSame('csrf-token', $request->cookies->get('csrf-token_'.$tokenValue)); + + $this->logger->expects($this->once())->method('warning')->with('CSRF validation failed: wrong token found in header info.'); + $this->assertFalse($csrfTokenManager->isTokenValid(new CsrfToken('test_token', str_repeat('b', 24)))); + } + + /** + * @testWith [0] + * [1] + * [2] + */ + public function testValidOriginMissingDoubleSubmit(int $checkHeader) + { + $csrfTokenManager = new SameOriginCsrfTokenManager($this->requestStack, $this->logger, null, [], $checkHeader); + + $request = new Request(); + $tokenValue = str_repeat('a', 24); + $request->headers->set('Origin', $request->getSchemeAndHttpHost()); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', $tokenValue); + + $this->logger->expects($this->once())->method('debug')->with('CSRF validation accepted using origin info.'); + $this->assertTrue($csrfTokenManager->isTokenValid($token)); + } + + public function testMissingEverything() + { + $request = new Request(); + $this->requestStack->push($request); + + $token = new CsrfToken('test_token', str_repeat('a', 24)); + + $this->logger->expects($this->once())->method('warning')->with('CSRF validation failed: double-submit and origin info not found.'); + $this->assertFalse($this->csrfTokenManager->isTokenValid($token)); + } + + public function testClearCookies() + { + $request = new Request([], [], ['csrf-token' => 2], ['csrf-token_test' => 'csrf-token']); + $response = new Response(); + + $this->csrfTokenManager->clearCookies($request, $response); + + $this->assertTrue($response->headers->has('Set-Cookie')); + } + + public function testPersistStrategyWithStartedSession() + { + $session = $this->createMock(Session::class); + $session->method('isStarted')->willReturn(true); + + $request = new Request(); + $request->setSession($session); + $request->attributes->set('csrf-token', 2 << 8); + + $session->expects($this->once())->method('set')->with('csrf-token', 2 << 8); + + $this->csrfTokenManager->persistStrategy($request); + } + + public function testPersistStrategyWithSessionNotStarted() + { + $session = $this->createMock(Session::class); + + $request = new Request(); + $request->setSession($session); + $request->attributes->set('csrf-token', 2 << 8); + + $session->expects($this->never())->method('set'); + + $this->csrfTokenManager->persistStrategy($request); + } + + public function testOnKernelResponse() + { + $request = new Request([], [], ['csrf-token' => 2], ['csrf-token_test' => 'csrf-token']); + $response = new Response(); + $event = new ResponseEvent($this->createMock(HttpKernelInterface::class), $request, HttpKernelInterface::MAIN_REQUEST, $response); + + $this->csrfTokenManager->onKernelResponse($event); + + $this->assertTrue($response->headers->has('Set-Cookie')); + } +} diff --git a/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php b/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php index 46cdb28..3ef3fce 100644 --- a/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php +++ b/Tests/TokenGenerator/UriSafeTokenGeneratorTest.php @@ -23,15 +23,10 @@ class UriSafeTokenGeneratorTest extends TestCase /** * A non alpha-numeric byte string. - * - * @var string */ - private static $bytes; + private static string $bytes; - /** - * @var UriSafeTokenGenerator - */ - private $generator; + private UriSafeTokenGenerator $generator; public static function setUpBeforeClass(): void { @@ -43,11 +38,6 @@ protected function setUp(): void $this->generator = new UriSafeTokenGenerator(self::ENTROPY); } - protected function tearDown(): void - { - $this->generator = null; - } - public function testGenerateToken() { $token = $this->generator->generateToken(); diff --git a/Tests/TokenStorage/NativeSessionTokenStorageTest.php b/Tests/TokenStorage/NativeSessionTokenStorageTest.php index 5e0383c..3c70c3c 100644 --- a/Tests/TokenStorage/NativeSessionTokenStorageTest.php +++ b/Tests/TokenStorage/NativeSessionTokenStorageTest.php @@ -26,10 +26,7 @@ class NativeSessionTokenStorageTest extends TestCase { private const SESSION_NAMESPACE = 'foobar'; - /** - * @var NativeSessionTokenStorage - */ - private $storage; + private NativeSessionTokenStorage $storage; protected function setUp(): void { diff --git a/Tests/TokenStorage/SessionTokenStorageTest.php b/Tests/TokenStorage/SessionTokenStorageTest.php index 6c9bf98..593d1a7 100644 --- a/Tests/TokenStorage/SessionTokenStorageTest.php +++ b/Tests/TokenStorage/SessionTokenStorageTest.php @@ -12,7 +12,6 @@ namespace Symfony\Component\Security\Csrf\Tests\TokenStorage; use PHPUnit\Framework\TestCase; -use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Session\Session; @@ -25,19 +24,10 @@ */ class SessionTokenStorageTest extends TestCase { - use ExpectDeprecationTrait; - private const SESSION_NAMESPACE = 'foobar'; - /** - * @var Session - */ - private $session; - - /** - * @var SessionTokenStorage - */ - private $storage; + private Session $session; + private SessionTokenStorage $storage; protected function setUp(): void { @@ -104,8 +94,10 @@ public function testGetNonExistingTokenFromClosedSession() public function testGetNonExistingTokenFromActiveSession() { - $this->expectException(TokenNotFoundException::class); $this->session->start(); + + $this->expectException(TokenNotFoundException::class); + $this->storage->getToken('token_id'); } @@ -162,50 +154,4 @@ public function testClearDoesNotRemoveNonNamespacedSessionValues() $this->assertTrue($this->session->has('foo')); $this->assertSame('baz', $this->session->get('foo')); } - - /** - * @group legacy - */ - public function testMockSessionIsCreatedWhenMissing() - { - $this->expectDeprecation('Since symfony/security-csrf 5.3: Using the "Symfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage" without a session has no effect and is deprecated. It will throw a "Symfony\Component\HttpFoundation\Exception\SessionNotFoundException" in Symfony 6.0'); - - $this->storage->setToken('token_id', 'TOKEN'); - - $requestStack = new RequestStack(); - $storage = new SessionTokenStorage($requestStack, self::SESSION_NAMESPACE); - - $this->assertFalse($storage->hasToken('foo')); - $storage->setToken('foo', 'bar'); - $this->assertTrue($storage->hasToken('foo')); - $this->assertSame('bar', $storage->getToken('foo')); - - $session = new Session(new MockArraySessionStorage()); - $request = new Request(); - $request->setSession($session); - $requestStack->push($request); - } - - /** - * @group legacy - */ - public function testMockSessionIsReusedEvenWhenRequestHasSession() - { - $this->expectDeprecation('Since symfony/security-csrf 5.3: Using the "Symfony\Component\Security\Csrf\TokenStorage\SessionTokenStorage" without a session has no effect and is deprecated. It will throw a "Symfony\Component\HttpFoundation\Exception\SessionNotFoundException" in Symfony 6.0'); - - $this->storage->setToken('token_id', 'TOKEN'); - - $requestStack = new RequestStack(); - $storage = new SessionTokenStorage($requestStack, self::SESSION_NAMESPACE); - - $storage->setToken('foo', 'bar'); - $this->assertSame('bar', $storage->getToken('foo')); - - $session = new Session(new MockArraySessionStorage()); - $request = new Request(); - $request->setSession($session); - $requestStack->push($request); - - $this->assertSame('bar', $storage->getToken('foo')); - } } diff --git a/TokenGenerator/TokenGeneratorInterface.php b/TokenGenerator/TokenGeneratorInterface.php index efb4360..9874092 100644 --- a/TokenGenerator/TokenGeneratorInterface.php +++ b/TokenGenerator/TokenGeneratorInterface.php @@ -20,8 +20,6 @@ interface TokenGeneratorInterface { /** * Generates a CSRF token. - * - * @return string */ - public function generateToken(); + public function generateToken(): string; } diff --git a/TokenGenerator/UriSafeTokenGenerator.php b/TokenGenerator/UriSafeTokenGenerator.php index 6ae2928..a91a4aa 100644 --- a/TokenGenerator/UriSafeTokenGenerator.php +++ b/TokenGenerator/UriSafeTokenGenerator.php @@ -18,26 +18,20 @@ */ class UriSafeTokenGenerator implements TokenGeneratorInterface { - private $entropy; - /** * Generates URI-safe CSRF tokens. * * @param int $entropy The amount of entropy collected for each token (in bits) */ - public function __construct(int $entropy = 256) - { + public function __construct( + private int $entropy = 256, + ) { if ($entropy <= 7) { throw new \InvalidArgumentException('Entropy should be greater than 7.'); } - - $this->entropy = $entropy; } - /** - * {@inheritdoc} - */ - public function generateToken() + public function generateToken(): string { // Generate an URI safe base64 encoded string that does not contain "+", // "/" or "=" which need to be URL encoded and make URLs unnecessarily diff --git a/TokenStorage/ClearableTokenStorageInterface.php b/TokenStorage/ClearableTokenStorageInterface.php index 0d6f16b..2f6d96b 100644 --- a/TokenStorage/ClearableTokenStorageInterface.php +++ b/TokenStorage/ClearableTokenStorageInterface.php @@ -19,5 +19,5 @@ interface ClearableTokenStorageInterface extends TokenStorageInterface /** * Removes all CSRF tokens. */ - public function clear(); + public function clear(): void; } diff --git a/TokenStorage/NativeSessionTokenStorage.php b/TokenStorage/NativeSessionTokenStorage.php index 97a95b6..4ab4782 100644 --- a/TokenStorage/NativeSessionTokenStorage.php +++ b/TokenStorage/NativeSessionTokenStorage.php @@ -25,23 +25,19 @@ class NativeSessionTokenStorage implements ClearableTokenStorageInterface */ public const SESSION_NAMESPACE = '_csrf'; - private $sessionStarted = false; - private $namespace; + private bool $sessionStarted = false; /** * Initializes the storage with a session namespace. * * @param string $namespace The namespace under which the token is stored in the session */ - public function __construct(string $namespace = self::SESSION_NAMESPACE) - { - $this->namespace = $namespace; + public function __construct( + private string $namespace = self::SESSION_NAMESPACE, + ) { } - /** - * {@inheritdoc} - */ - public function getToken(string $tokenId) + public function getToken(string $tokenId): string { if (!$this->sessionStarted) { $this->startSession(); @@ -54,10 +50,7 @@ public function getToken(string $tokenId) return (string) $_SESSION[$this->namespace][$tokenId]; } - /** - * {@inheritdoc} - */ - public function setToken(string $tokenId, string $token) + public function setToken(string $tokenId, #[\SensitiveParameter] string $token): void { if (!$this->sessionStarted) { $this->startSession(); @@ -66,10 +59,7 @@ public function setToken(string $tokenId, string $token) $_SESSION[$this->namespace][$tokenId] = $token; } - /** - * {@inheritdoc} - */ - public function hasToken(string $tokenId) + public function hasToken(string $tokenId): bool { if (!$this->sessionStarted) { $this->startSession(); @@ -78,10 +68,7 @@ public function hasToken(string $tokenId) return isset($_SESSION[$this->namespace][$tokenId]); } - /** - * {@inheritdoc} - */ - public function removeToken(string $tokenId) + public function removeToken(string $tokenId): ?string { if (!$this->sessionStarted) { $this->startSession(); @@ -102,15 +89,12 @@ public function removeToken(string $tokenId) return $token; } - /** - * {@inheritdoc} - */ - public function clear() + public function clear(): void { unset($_SESSION[$this->namespace]); } - private function startSession() + private function startSession(): void { if (\PHP_SESSION_NONE === session_status()) { session_start(); diff --git a/TokenStorage/SessionTokenStorage.php b/TokenStorage/SessionTokenStorage.php index 881f0fd..d5614bf 100644 --- a/TokenStorage/SessionTokenStorage.php +++ b/TokenStorage/SessionTokenStorage.php @@ -12,11 +12,8 @@ namespace Symfony\Component\Security\Csrf\TokenStorage; use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException; -use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; -use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\SessionInterface; -use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; use Symfony\Component\Security\Csrf\Exception\TokenNotFoundException; /** @@ -31,37 +28,18 @@ class SessionTokenStorage implements ClearableTokenStorageInterface */ public const SESSION_NAMESPACE = '_csrf'; - private $requestStack; - private $namespace; - /** - * To be removed in Symfony 6.0. - */ - private $session; - /** * Initializes the storage with a RequestStack object and a session namespace. * - * @param RequestStack $requestStack - * @param string $namespace The namespace under which the token is stored in the requestStack + * @param string $namespace The namespace under which the token is stored in the requestStack */ - public function __construct(/* RequestStack */ $requestStack, string $namespace = self::SESSION_NAMESPACE) - { - if ($requestStack instanceof SessionInterface) { - trigger_deprecation('symfony/security-csrf', '5.3', 'Passing a "%s" to "%s" is deprecated, use a "%s" instead.', SessionInterface::class, __CLASS__, RequestStack::class); - $request = new Request(); - $request->setSession($requestStack); - - $requestStack = new RequestStack(); - $requestStack->push($request); - } - $this->requestStack = $requestStack; - $this->namespace = $namespace; + public function __construct( + private RequestStack $requestStack, + private string $namespace = self::SESSION_NAMESPACE, + ) { } - /** - * {@inheritdoc} - */ - public function getToken(string $tokenId) + public function getToken(string $tokenId): string { $session = $this->getSession(); if (!$session->isStarted()) { @@ -75,10 +53,7 @@ public function getToken(string $tokenId) return (string) $session->get($this->namespace.'/'.$tokenId); } - /** - * {@inheritdoc} - */ - public function setToken(string $tokenId, string $token) + public function setToken(string $tokenId, #[\SensitiveParameter] string $token): void { $session = $this->getSession(); if (!$session->isStarted()) { @@ -88,10 +63,7 @@ public function setToken(string $tokenId, string $token) $session->set($this->namespace.'/'.$tokenId, $token); } - /** - * {@inheritdoc} - */ - public function hasToken(string $tokenId) + public function hasToken(string $tokenId): bool { $session = $this->getSession(); if (!$session->isStarted()) { @@ -101,10 +73,7 @@ public function hasToken(string $tokenId) return $session->has($this->namespace.'/'.$tokenId); } - /** - * {@inheritdoc} - */ - public function removeToken(string $tokenId) + public function removeToken(string $tokenId): ?string { $session = $this->getSession(); if (!$session->isStarted()) { @@ -114,10 +83,7 @@ public function removeToken(string $tokenId) return $session->remove($this->namespace.'/'.$tokenId); } - /** - * {@inheritdoc} - */ - public function clear() + public function clear(): void { $session = $this->getSession(); foreach (array_keys($session->all()) as $key) { @@ -127,14 +93,11 @@ public function clear() } } + /** + * @throws SessionNotFoundException + */ private function getSession(): SessionInterface { - try { - return $this->session ?? $this->requestStack->getSession(); - } catch (SessionNotFoundException $e) { - trigger_deprecation('symfony/security-csrf', '5.3', 'Using the "%s" without a session has no effect and is deprecated. It will throw a "%s" in Symfony 6.0', __CLASS__, SessionNotFoundException::class); - - return $this->session ?? $this->session = new Session(new MockArraySessionStorage()); - } + return $this->requestStack->getSession(); } } diff --git a/TokenStorage/TokenStorageInterface.php b/TokenStorage/TokenStorageInterface.php index 15d8bbd..804b6a4 100644 --- a/TokenStorage/TokenStorageInterface.php +++ b/TokenStorage/TokenStorageInterface.php @@ -21,16 +21,14 @@ interface TokenStorageInterface /** * Reads a stored CSRF token. * - * @return string - * * @throws \Symfony\Component\Security\Csrf\Exception\TokenNotFoundException If the token ID does not exist */ - public function getToken(string $tokenId); + public function getToken(string $tokenId): string; /** * Stores a CSRF token. */ - public function setToken(string $tokenId, string $token); + public function setToken(string $tokenId, #[\SensitiveParameter] string $token): void; /** * Removes a CSRF token. @@ -38,12 +36,10 @@ public function setToken(string $tokenId, string $token); * @return string|null Returns the removed token if one existed, NULL * otherwise */ - public function removeToken(string $tokenId); + public function removeToken(string $tokenId): ?string; /** * Checks whether a token with the given token ID exists. - * - * @return bool */ - public function hasToken(string $tokenId); + public function hasToken(string $tokenId): bool; } diff --git a/composer.json b/composer.json index 0ba322d..c2bfed1 100644 --- a/composer.json +++ b/composer.json @@ -16,19 +16,16 @@ } ], "require": { - "php": ">=7.2.5", - "symfony/deprecation-contracts": "^2.1|^3", - "symfony/polyfill-php80": "^1.16", - "symfony/security-core": "^4.4|^5.0|^6.0" + "php": ">=8.2", + "symfony/security-core": "^6.4|^7.0" }, "require-dev": { - "symfony/http-foundation": "^5.3|^6.0" + "psr/log": "^1|^2|^3", + "symfony/http-foundation": "^6.4|^7.0", + "symfony/http-kernel": "^6.4|^7.0" }, "conflict": { - "symfony/http-foundation": "<5.3" - }, - "suggest": { - "symfony/http-foundation": "For using the class SessionTokenStorage." + "symfony/http-foundation": "<6.4" }, "autoload": { "psr-4": { "Symfony\\Component\\Security\\Csrf\\": "" }, diff --git a/phpunit.xml.dist b/phpunit.xml.dist index c37ee14..012cb73 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,7 +1,7 @@ - - + + ./ - - ./Tests - ./vendor - - - + + + ./Tests + ./vendor + +