From a0535d866fa713b6283c16cbb097daf3486d8ac9 Mon Sep 17 00:00:00 2001 From: NGUEREZA Tony Date: Thu, 2 Nov 2023 05:31:29 +0100 Subject: [PATCH] Update CSRF feature --- src/Http/Middleware/CsrfMiddleware.php | 6 +- src/Security/Csrf/CsrfManager.php | 8 -- .../Csrf/Storage/CsrfSessionStorage.php | 2 +- .../Csrf/Storage/CsrfUserSessionStorage.php | 75 +++++++++++++++++ tests/Http/Middleware/CsrfMiddlewareTest.php | 32 ++++++++ .../Storage/CsrfUserSessionStorageTest.php | 81 +++++++++++++++++++ 6 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 src/Security/Csrf/Storage/CsrfUserSessionStorage.php create mode 100644 tests/Security/Csrf/Storage/CsrfUserSessionStorageTest.php diff --git a/src/Http/Middleware/CsrfMiddleware.php b/src/Http/Middleware/CsrfMiddleware.php index 8b5b373..bc035a3 100644 --- a/src/Http/Middleware/CsrfMiddleware.php +++ b/src/Http/Middleware/CsrfMiddleware.php @@ -49,7 +49,6 @@ namespace Platine\Framework\Http\Middleware; use Platine\Config\Config; -use Platine\Framework\Http\RequestData; use Platine\Framework\Security\Csrf\CsrfManager; use Platine\Http\Handler\MiddlewareInterface; use Platine\Http\Handler\RequestHandlerInterface; @@ -160,9 +159,12 @@ protected function shouldBeProcessed(ServerRequestInterface $request): bool return false; } + // Check for route attribute (useful for GET method) + $csrf = (bool) $route->getAttribute('csrf'); + $methods = $this->config->get('security.csrf.http_methods', []); - if (!in_array($request->getMethod(), $methods)) { + if (!in_array($request->getMethod(), $methods) && $csrf === false) { return false; } diff --git a/src/Security/Csrf/CsrfManager.php b/src/Security/Csrf/CsrfManager.php index bc45047..40d0d3a 100644 --- a/src/Security/Csrf/CsrfManager.php +++ b/src/Security/Csrf/CsrfManager.php @@ -135,14 +135,6 @@ public function getToken(?string $key = null): string if ($key === null) { $key = $this->getConfigValue('key'); } - - if ($this->unique === false) { - $value = sha1(Str::randomToken(24)); - $expire = $this->getConfigValue('expire') ?? 300; - $expireTime = time() + $expire; - - $this->storage->set($key, $value, $expireTime); - } $value = $this->storage->get($key); if ($value === null) { diff --git a/src/Security/Csrf/Storage/CsrfSessionStorage.php b/src/Security/Csrf/Storage/CsrfSessionStorage.php index ce1ecad..c1fffee 100644 --- a/src/Security/Csrf/Storage/CsrfSessionStorage.php +++ b/src/Security/Csrf/Storage/CsrfSessionStorage.php @@ -127,7 +127,7 @@ public function clear(): void * @param string $name * @return string */ - private function getKeyName(string $name): string + protected function getKeyName(string $name): string { return sprintf('%s.%s', self::CSRF_SESSION_KEY, $name); } diff --git a/src/Security/Csrf/Storage/CsrfUserSessionStorage.php b/src/Security/Csrf/Storage/CsrfUserSessionStorage.php new file mode 100644 index 0000000..20e4590 --- /dev/null +++ b/src/Security/Csrf/Storage/CsrfUserSessionStorage.php @@ -0,0 +1,75 @@ +getKeyName($name); + $token = $this->session->get($key, null); + + return $token; + } + + /** + * {@inheritdoc} + */ + public function set(string $name, string $token, int $expire): void + { + $key = $this->getKeyName($name); + $this->session->set($key, $token); + } +} diff --git a/tests/Http/Middleware/CsrfMiddlewareTest.php b/tests/Http/Middleware/CsrfMiddlewareTest.php index 923f333..ce49d7c 100644 --- a/tests/Http/Middleware/CsrfMiddlewareTest.php +++ b/tests/Http/Middleware/CsrfMiddlewareTest.php @@ -156,4 +156,36 @@ public function testProcessSuccess(): void $this->assertEquals(0, $res->getStatusCode()); } + + public function testProcessSuccessUsingGetMethod(): void + { + $route = $this->getMockInstance(Route::class, [ + 'getPattern' => '/bar/foo', + 'getAttribute' => true, + ]); + $request = $this->getMockInstance(ServerRequest::class, [ + 'getAttribute' => $route, + 'getMethod' => 'GET', + 'getQueryParams' => ['csrf_key' => 'foo'], + ]); + $handler = $this->getMockInstance(HttpKernel::class); + + $manager = $this->getMockInstance(CsrfManager::class, [ + 'validate' => true + ]); + $lang = $this->getMockInstance(Lang::class); + $logger = $this->getMockInstance(Logger::class); + $config = $this->getMockInstanceMap(Config::class, [ + 'get' => [ + ['security.csrf.http_methods', [], ['POST']], + ['security.csrf.url_whitelist', [], ['/api']], + ['security.csrf.key', '', 'csrf_key'], + ] + ]); + + $o = new CsrfMiddleware($logger, $lang, $config, $manager); + $res = $o->process($request, $handler); + + $this->assertEquals(0, $res->getStatusCode()); + } } diff --git a/tests/Security/Csrf/Storage/CsrfUserSessionStorageTest.php b/tests/Security/Csrf/Storage/CsrfUserSessionStorageTest.php new file mode 100644 index 0000000..4047f2f --- /dev/null +++ b/tests/Security/Csrf/Storage/CsrfUserSessionStorageTest.php @@ -0,0 +1,81 @@ +assertInstanceOf(CsrfUserSessionStorage::class, $o); + } + + public function testGetNull(): void + { + $session = new Session(); + + $o = new CsrfUserSessionStorage($session); + + $this->assertNull($o->get('token')); + } + + public function testGetDeleteClearSuccess(): void + { + $_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY] = ['token' => 'foobar']; + $session = new Session(); + + $o = new CsrfUserSessionStorage($session); + + $res = $o->get('token'); + $this->assertEquals('foobar', $res); + $this->assertCount(1, $_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + $o->delete('token'); + + $this->assertNull($o->get('token')); + $this->assertIsArray($_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + $this->assertCount(0, $_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + + $o->clear(); + $this->assertNull($o->get('token')); + $this->assertArrayNotHasKey(CsrfSessionStorage::CSRF_SESSION_KEY, $_SESSION); + } + + public function testSet(): void + { + $session = new Session(); + + $o = new CsrfUserSessionStorage($session); + + $this->assertNull($o->get('token')); + $this->assertArrayNotHasKey(CsrfSessionStorage::CSRF_SESSION_KEY, $_SESSION); + + $o->set('token', 'foobar', time() + 100); + + $res = $o->get('token'); + $this->assertEquals('foobar', $res); + $this->assertCount(1, $_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + $o->delete('token'); + + $this->assertNull($o->get('token')); + $this->assertIsArray($_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + $this->assertCount(0, $_SESSION[CsrfSessionStorage::CSRF_SESSION_KEY]); + + $o->clear(); + $this->assertNull($o->get('token')); + $this->assertArrayNotHasKey(CsrfSessionStorage::CSRF_SESSION_KEY, $_SESSION); + } +}