From 38ea3c5c3a40fa8af9efa01a122fecce44725a3a Mon Sep 17 00:00:00 2001 From: Eugen Gorbunov Date: Thu, 7 Nov 2024 13:39:20 +0400 Subject: [PATCH] Fix #71: Deprecate `CsrfMiddleware` in favor of `CsrfTokenMiddleware` --- CHANGELOG.md | 2 +- README.md | 8 +- src/CsrfMiddleware.php | 1 + src/CsrfTokenMiddleware.php | 109 ++++++++++ tests/CsrfMiddlewareTest.php | 10 +- tests/DeprecatedCsrfMiddlewareTest.php | 61 ++++++ tests/DeprecatedTokenCsrfMiddlewareTest.php | 196 ++++++++++++++++++ .../DeprecatedHmacTokenCsrfMiddlewareTest.php | 22 ++ ...tedSynchronizerTokenCsrfMiddlewareTest.php | 52 +++++ tests/TokenCsrfMiddlewareTest.php | 8 +- 10 files changed, 455 insertions(+), 14 deletions(-) create mode 100644 src/CsrfTokenMiddleware.php create mode 100644 tests/DeprecatedCsrfMiddlewareTest.php create mode 100644 tests/DeprecatedTokenCsrfMiddlewareTest.php create mode 100644 tests/Hmac/DeprecatedHmacTokenCsrfMiddlewareTest.php create mode 100644 tests/Synchronizer/DeprecatedSynchronizerTokenCsrfMiddlewareTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d417dd3..1077f9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 2.1.2 under development -- no changes in this release. +- Chg #71: Deprecate `CsrfMiddleware` in favor of `CsrfTokenMiddleware` (@ev-gor) ## 2.1.1 May 08, 2024 diff --git a/README.md b/README.md index 655ff66..1b9a84c 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ composer require yiisoft/csrf ## General usage -In order to enable CSRF protection you need to add `CsrfMiddleware` to your main middleware stack. +In order to enable CSRF protection you need to add `CsrfTokenMiddleware` to your main middleware stack. In Yii it is done by configuring `config/web/application.php`: ```php @@ -48,7 +48,7 @@ return [ [ ErrorCatcher::class, SessionMiddleware::class, - CsrfMiddleware::class, // <-- add this + CsrfTokenMiddleware::class, // <-- add this Router::class, ] ); @@ -74,7 +74,7 @@ You can change this behavior by implementing your own request handler: use Psr\Http\Message\ResponseFactoryInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Yiisoft\Csrf\CsrfMiddleware; +use Yiisoft\Csrf\CsrfTokenMiddleware; /** * @var Psr\Http\Message\ResponseFactoryInterface $responseFactory @@ -99,7 +99,7 @@ $failureHandler = new class ($responseFactory) implements RequestHandlerInterfac } }; -$middleware = new CsrfMiddleware($responseFactory, $csrfToken, $failureHandler); +$middleware = new CsrfTokenMiddleware($responseFactory, $csrfToken, $failureHandler); ``` ## CSRF Tokens diff --git a/src/CsrfMiddleware.php b/src/CsrfMiddleware.php index cffab6c..679ff1d 100644 --- a/src/CsrfMiddleware.php +++ b/src/CsrfMiddleware.php @@ -19,6 +19,7 @@ * PSR-15 middleware that takes care of token validation. * * @link https://www.php-fig.org/psr/psr-15/ + * @deprecated Use the {@see CsrfTokenMiddleware} class instead. */ final class CsrfMiddleware implements MiddlewareInterface { diff --git a/src/CsrfTokenMiddleware.php b/src/CsrfTokenMiddleware.php new file mode 100644 index 0000000..0ef2b3b --- /dev/null +++ b/src/CsrfTokenMiddleware.php @@ -0,0 +1,109 @@ +responseFactory = $responseFactory; + $this->token = $token; + $this->failureHandler = $failureHandler; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + if ($this->validateCsrfToken($request)) { + return $handler->handle($request); + } + + if ($this->failureHandler !== null) { + return $this->failureHandler->handle($request); + } + + $response = $this->responseFactory->createResponse(Status::UNPROCESSABLE_ENTITY); + $response + ->getBody() + ->write(Status::TEXTS[Status::UNPROCESSABLE_ENTITY]); + return $response; + } + + public function withParameterName(string $name): self + { + $new = clone $this; + $new->parameterName = $name; + return $new; + } + + public function withHeaderName(string $name): self + { + $new = clone $this; + $new->headerName = $name; + return $new; + } + + public function getParameterName(): string + { + return $this->parameterName; + } + + public function getHeaderName(): string + { + return $this->headerName; + } + + private function validateCsrfToken(ServerRequestInterface $request): bool + { + if (in_array($request->getMethod(), [Method::GET, Method::HEAD, Method::OPTIONS], true)) { + return true; + } + + $token = $this->getTokenFromRequest($request); + + return !empty($token) && $this->token->validate($token); + } + + private function getTokenFromRequest(ServerRequestInterface $request): ?string + { + $parsedBody = $request->getParsedBody(); + + $token = $parsedBody[$this->parameterName] ?? null; + if (empty($token)) { + $headers = $request->getHeader($this->headerName); + $token = reset($headers); + } + + return is_string($token) ? $token : null; + } +} diff --git a/tests/CsrfMiddlewareTest.php b/tests/CsrfMiddlewareTest.php index 95a83fd..cc0847e 100644 --- a/tests/CsrfMiddlewareTest.php +++ b/tests/CsrfMiddlewareTest.php @@ -6,7 +6,7 @@ use Nyholm\Psr7\Factory\Psr17Factory; use PHPUnit\Framework\TestCase; -use Yiisoft\Csrf\CsrfMiddleware; +use Yiisoft\Csrf\CsrfTokenMiddleware; use Yiisoft\Csrf\Synchronizer\Generator\RandomCsrfTokenGenerator; use Yiisoft\Csrf\Synchronizer\SynchronizerCsrfToken; use Yiisoft\Csrf\Tests\Synchronizer\Storage\MockCsrfTokenStorage; @@ -16,7 +16,7 @@ final class CsrfMiddlewareTest extends TestCase public function testDefaultParameterName(): void { $middleware = $this->createMiddleware(); - $this->assertSame(CsrfMiddleware::PARAMETER_NAME, $middleware->getParameterName()); + $this->assertSame(CsrfTokenMiddleware::PARAMETER_NAME, $middleware->getParameterName()); } public function testGetParameterName(): void @@ -30,7 +30,7 @@ public function testGetParameterName(): void public function testDefaultHeaderName(): void { $middleware = $this->createMiddleware(); - $this->assertSame(CsrfMiddleware::HEADER_NAME, $middleware->getHeaderName()); + $this->assertSame(CsrfTokenMiddleware::HEADER_NAME, $middleware->getHeaderName()); } public function testGetHeaderName(): void @@ -48,9 +48,9 @@ public function testImmutability(): void $this->assertNotSame($original, $original->withParameterName('csrf')); } - private function createMiddleware(): CsrfMiddleware + private function createMiddleware(): CsrfTokenMiddleware { - return new CsrfMiddleware( + return new CsrfTokenMiddleware( new Psr17Factory(), new SynchronizerCsrfToken( new RandomCsrfTokenGenerator(), diff --git a/tests/DeprecatedCsrfMiddlewareTest.php b/tests/DeprecatedCsrfMiddlewareTest.php new file mode 100644 index 0000000..4552f09 --- /dev/null +++ b/tests/DeprecatedCsrfMiddlewareTest.php @@ -0,0 +1,61 @@ +createMiddleware(); + $this->assertSame(CsrfMiddleware::PARAMETER_NAME, $middleware->getParameterName()); + } + + public function testGetParameterName(): void + { + $middleware = $this + ->createMiddleware() + ->withParameterName('my-csrf'); + $this->assertSame('my-csrf', $middleware->getParameterName()); + } + + public function testDefaultHeaderName(): void + { + $middleware = $this->createMiddleware(); + $this->assertSame(CsrfMiddleware::HEADER_NAME, $middleware->getHeaderName()); + } + + public function testGetHeaderName(): void + { + $middleware = $this + ->createMiddleware() + ->withHeaderName('MY-CSRF'); + $this->assertSame('MY-CSRF', $middleware->getHeaderName()); + } + + public function testImmutability(): void + { + $original = $this->createMiddleware(); + $this->assertNotSame($original, $original->withHeaderName('csrf')); + $this->assertNotSame($original, $original->withParameterName('csrf')); + } + + private function createMiddleware(): CsrfMiddleware + { + return new CsrfMiddleware( + new Psr17Factory(), + new SynchronizerCsrfToken( + new RandomCsrfTokenGenerator(), + new MockCsrfTokenStorage() + ) + ); + } +} diff --git a/tests/DeprecatedTokenCsrfMiddlewareTest.php b/tests/DeprecatedTokenCsrfMiddlewareTest.php new file mode 100644 index 0000000..c00dd65 --- /dev/null +++ b/tests/DeprecatedTokenCsrfMiddlewareTest.php @@ -0,0 +1,196 @@ +createCsrfMiddleware(); + $response = $middleware->process( + $this->createPostServerRequestWithBodyToken($this->token), + $this->createRequestHandler() + ); + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testValidTokenInBodyPutRequestResultIn200(): void + { + $middleware = $this->createCsrfMiddleware(); + $response = $middleware->process( + $this->createPutServerRequestWithBodyToken($this->token), + $this->createRequestHandler() + ); + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testValidTokenInBodyDeleteRequestResultIn200(): void + { + $middleware = $this->createCsrfMiddleware(); + $response = $middleware->process( + $this->createDeleteServerRequestWithBodyToken($this->token), + $this->createRequestHandler() + ); + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testValidTokenInHeaderResultIn200(): void + { + $middleware = $this->createCsrfMiddleware(); + $response = $middleware->process( + $this->createPostServerRequestWithHeaderToken($this->token), + $this->createRequestHandler() + ); + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testValidTokenInCustomHeaderResultIn200(): void + { + $headerName = 'CUSTOM-CSRF'; + + $middleware = $this + ->createCsrfMiddleware() + ->withHeaderName($headerName); + $response = $middleware->process( + $this->createPostServerRequestWithHeaderToken($this->token, $headerName), + $this->createRequestHandler() + ); + + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testGetIsAlwaysAllowed(): void + { + $middleware = $this->createCsrfMiddleware(); + $response = $middleware->process($this->createServerRequest(Method::GET), $this->createRequestHandler()); + $this->assertEquals(200, $response->getStatusCode()); + } + + public function testInvalidTokenResultIn422(): void + { + $middleware = $this->createCsrfMiddleware(); + + $response = $middleware->process( + $this->createPostServerRequestWithBodyToken(Random::string()), + $this->createRequestHandler() + ); + + $this->assertEquals(Status::TEXTS[Status::UNPROCESSABLE_ENTITY], $response->getBody()); + $this->assertEquals(Status::UNPROCESSABLE_ENTITY, $response->getStatusCode()); + } + + public function testInvalidTokenResultWithCustomFailureHandler(): void + { + $failureHandler = new class () implements RequestHandlerInterface { + public function handle(ServerRequestInterface $request): ResponseInterface + { + $response = new Response(Status::BAD_REQUEST); + $response + ->getBody() + ->write(Status::TEXTS[Status::BAD_REQUEST]); + return $response; + } + }; + + $middleware = $this->createCsrfMiddleware(null, $failureHandler); + + $response = $middleware->process( + $this->createPostServerRequestWithBodyToken(Random::string()), + $this->createRequestHandler(), + ); + + $this->assertEquals(Status::TEXTS[Status::BAD_REQUEST], $response->getBody()); + $this->assertEquals(Status::BAD_REQUEST, $response->getStatusCode()); + } + + public function testEmptyTokenInRequestResultIn422(): void + { + $middleware = $this->createCsrfMiddleware(); + $response = $middleware->process($this->createServerRequest(), $this->createRequestHandler()); + $this->assertEquals(Status::UNPROCESSABLE_ENTITY, $response->getStatusCode()); + } + + private function createServerRequest( + string $method = Method::POST, + array $bodyParams = [], + array $headParams = [] + ): ServerRequestInterface { + $request = new ServerRequest($method, '/', $headParams); + return $request->withParsedBody($bodyParams); + } + + protected function createPostServerRequestWithBodyToken(string $token): ServerRequestInterface + { + return $this->createServerRequest(Method::POST, $this->getBodyRequestParamsByToken($token)); + } + + private function createPutServerRequestWithBodyToken(string $token): ServerRequestInterface + { + return $this->createServerRequest(Method::PUT, $this->getBodyRequestParamsByToken($token)); + } + + private function createDeleteServerRequestWithBodyToken(string $token): ServerRequestInterface + { + return $this->createServerRequest(Method::DELETE, $this->getBodyRequestParamsByToken($token)); + } + + private function createPostServerRequestWithHeaderToken( + string $token, + string $headerName = CsrfMiddleware::HEADER_NAME + ): ServerRequestInterface { + return $this->createServerRequest(Method::POST, [], [ + $headerName => $token, + ]); + } + + protected function createRequestHandler(): RequestHandlerInterface + { + $requestHandler = $this->createMock(RequestHandlerInterface::class); + $requestHandler + ->method('handle') + ->willReturn(new Response(200)); + + return $requestHandler; + } + + private function getBodyRequestParamsByToken(string $token): array + { + return [ + self::PARAM_NAME => $token, + ]; + } + + protected function createCsrfMiddleware( + ?CsrfTokenInterface $csrfToken = null, + RequestHandlerInterface $failureHandler = null + ): CsrfMiddleware { + $csrfToken = new MaskedCsrfToken($csrfToken ?? $this->createCsrfToken()); + $this->token = $csrfToken->getValue(); + + $middleware = new CsrfMiddleware(new Psr17Factory(), $csrfToken, $failureHandler); + + return $middleware->withParameterName(self::PARAM_NAME); + } + + abstract protected function createCsrfToken(): CsrfTokenInterface; +} diff --git a/tests/Hmac/DeprecatedHmacTokenCsrfMiddlewareTest.php b/tests/Hmac/DeprecatedHmacTokenCsrfMiddlewareTest.php new file mode 100644 index 0000000..ed49236 --- /dev/null +++ b/tests/Hmac/DeprecatedHmacTokenCsrfMiddlewareTest.php @@ -0,0 +1,22 @@ +createCsrfMiddleware( + new SynchronizerCsrfToken( + new RandomCsrfTokenGenerator(), + new MockCsrfTokenStorage() + ) + ); + + $response = $middleware->process( + $this->createPostServerRequestWithBodyToken(Random::string()), + $this->createRequestHandler() + ); + + $this->assertEquals(422, $response->getStatusCode()); + } + + protected function createCsrfToken(): CsrfTokenInterface + { + return new SynchronizerCsrfToken( + new RandomCsrfTokenGenerator(), + $this->createStorageMock(Random::string()) + ); + } + + private function createStorageMock(string $returnToken): CsrfTokenStorageInterface + { + $mock = $this->createMock(MockCsrfTokenStorage::class); + + $mock + ->method('get') + ->willReturn($returnToken); + + return $mock; + } +} diff --git a/tests/TokenCsrfMiddlewareTest.php b/tests/TokenCsrfMiddlewareTest.php index b355f8c..fa4224b 100644 --- a/tests/TokenCsrfMiddlewareTest.php +++ b/tests/TokenCsrfMiddlewareTest.php @@ -11,7 +11,7 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Server\RequestHandlerInterface; -use Yiisoft\Csrf\CsrfMiddleware; +use Yiisoft\Csrf\CsrfTokenMiddleware; use Yiisoft\Csrf\CsrfTokenInterface; use Yiisoft\Csrf\MaskedCsrfToken; use Yiisoft\Http\Method; @@ -156,7 +156,7 @@ private function createDeleteServerRequestWithBodyToken(string $token): ServerRe private function createPostServerRequestWithHeaderToken( string $token, - string $headerName = CsrfMiddleware::HEADER_NAME + string $headerName = CsrfTokenMiddleware::HEADER_NAME ): ServerRequestInterface { return $this->createServerRequest(Method::POST, [], [ $headerName => $token, @@ -183,11 +183,11 @@ private function getBodyRequestParamsByToken(string $token): array protected function createCsrfMiddleware( ?CsrfTokenInterface $csrfToken = null, RequestHandlerInterface $failureHandler = null - ): CsrfMiddleware { + ): CsrfTokenMiddleware { $csrfToken = new MaskedCsrfToken($csrfToken ?? $this->createCsrfToken()); $this->token = $csrfToken->getValue(); - $middleware = new CsrfMiddleware(new Psr17Factory(), $csrfToken, $failureHandler); + $middleware = new CsrfTokenMiddleware(new Psr17Factory(), $csrfToken, $failureHandler); return $middleware->withParameterName(self::PARAM_NAME); }