Update CSRF feature

platine-php · Oct 31, 2023 · 7effb3b · 7effb3b
1 parent 78e2ce7
commit 7effb3b
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 139 deletions.
diff --git a/src/Http/Response/RestResponse.php b/src/Http/Response/RestResponse.php
@@ -81,8 +81,8 @@ public function __construct(
         if (!empty($message)) {
             $result['message'] = $message;
         }
-		
-		$result['data'] = $data;
+
+        $result['data'] = $data;
 
         if (!empty($extras)) {
             $result = array_merge($result, $extras);

diff --git a/src/Security/Csrf/CsrfManager.php b/src/Security/Csrf/CsrfManager.php
@@ -50,6 +50,7 @@
 use Platine\Config\Config;
 use Platine\Framework\Http\RequestData;
 use Platine\Framework\Security\Csrf\CsrfStorageInterface;
+use Platine\Framework\Security\Csrf\Storage\CsrfNullStorage;
 use Platine\Http\ServerRequestInterface;
 use Platine\Stdlib\Helper\Str;
 
@@ -102,8 +103,8 @@ public function validate(ServerRequestInterface $request, ?string $key = null):
             $key = $this->getConfigValue('key');
         }
 
-        $data = $this->storage->get($key);
-        if ($data === null || $data['expire'] <= time()) {
+        $storageToken = $this->storage->get($key);
+        if ($storageToken === null) {
             return false;
         }
 
@@ -113,7 +114,7 @@ public function validate(ServerRequestInterface $request, ?string $key = null):
             $token = $param->get($key);
         }
 
-        if ($token === null || $token !== $data['value']) {
+        if ($token === null || $token !== $storageToken) {
             return false;
         }
 
@@ -135,22 +136,17 @@ public function getToken(?string $key = null): string
             $key = $this->getConfigValue('key');
         }
 
-        $data = $this->storage->get($key);
-        if ($data === null) {
-            // Generate
+        $value = $this->storage->get($key);
+        if ($value === null) {
+            // Generate the token
             $value = sha1(Str::randomToken(24));
             $expire = $this->getConfigValue('expire') ?? 300;
             $expireTime = time() + $expire;
 
-            $data = [
-                'expire' => $expireTime,
-                'value' => $value,
-            ];
-
-            $this->storage->set($key, $data);
+            $this->storage->set($key, $value, $expireTime);
         }
 
-        return $data['value'];
+        return $value;
     }
 
     /**

diff --git a/src/Security/Csrf/CsrfStorageInterface.php b/src/Security/Csrf/CsrfStorageInterface.php
@@ -56,17 +56,18 @@ interface CsrfStorageInterface
     /**
      * Return the token information from storage
      * @param string $name
-     * @return array<string, int|string>|null
+     * @return string|null
      */
-    public function get(string $name): ?array;
+    public function get(string $name): ?string;
 
     /**
      * Set the token information into storage
      * @param string $name
-     * @param array<string, int|string> $data
+     * @param string $token
+     * @param int $expire
      * @return void
      */
-    public function set(string $name, array $data): void;
+    public function set(string $name, string $token, int $expire): void;
 
     /**
      * Delete token information from storage

diff --git a/src/Security/Csrf/Storage/CsrfNullStorage.php b/src/Security/Csrf/Storage/CsrfNullStorage.php
@@ -64,17 +64,17 @@ class CsrfNullStorage implements CsrfStorageInterface
     /**
      * {@inheritdoc}
      */
-    public function get(string $name): ?array
+    public function get(string $name): ?string
     {
         return $this->data[$name] ?? null;
     }
 
     /**
      * {@inheritdoc}
      */
-    public function set(string $name, array $data): void
+    public function set(string $name, string $token, int $expire): void
     {
-         $this->data[$name] = $data;
+         $this->data[$name] = $token;
     }
 
     /**

diff --git a/src/Security/Csrf/Storage/CsrfSessionStorage.php b/src/Security/Csrf/Storage/CsrfSessionStorage.php
@@ -81,19 +81,28 @@ public function __construct(Session $session)
     /**
      * {@inheritdoc}
      */
-    public function get(string $name): ?array
+    public function get(string $name): ?string
     {
         $key = $this->getKeyName($name);
-        return $this->session->get($key);
+        $data = $this->session->get($key, []);
+
+        if (count($data) === 0 || $data['expire'] <= time()) {
+            return null;
+        }
+
+        return $data['value'];
     }
 
     /**
      * {@inheritdoc}
      */
-    public function set(string $name, array $data): void
+    public function set(string $name, string $token, int $expire): void
     {
         $key = $this->getKeyName($name);
-        $this->session->set($key, $data);
+        $this->session->set($key, [
+            'value' => $token,
+            'expire' => $expire,
+        ]);
     }
 
     /**

diff --git a/tests/Security/Csrf/CsrfManagerTest.php b/tests/Security/Csrf/CsrfManagerTest.php
@@ -20,16 +20,16 @@ public function testConstruct(): void
     {
         $storage = new CsrfNullStorage();
         $config = $this->getMockInstance(Config::class);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $this->assertInstanceOf(CsrfManager::class, $o);
     }
-    
+
     public function testGetToken(): void
     {
         global $mock_sha1_foo;
-        
+
         $mock_sha1_foo = true;
         $storage = new CsrfNullStorage();
         $config = $this->getMockInstance(Config::class, [
@@ -38,12 +38,12 @@ public function testGetToken(): void
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $this->assertEquals('foo', $o->getToken());
     }
-    
+
     public function testValidateStorageReturnNull(): void
     {
         $storage = new CsrfNullStorage();
@@ -53,127 +53,110 @@ public function testValidateStorageReturnNull(): void
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class);
         $this->assertFalse($o->validate($request));
     }
-    
+
     public function testValidateTokenAlreadyExpired(): void
     {
         $storage = new CsrfNullStorage();
-        $storage->set('csrf_key', [
-            'value' => 'bar',
-            'expire' => time() - 100,
-        ]);
+        $storage->set('csrf_key', 'bar', time() - 100);
         $config = $this->getMockInstance(Config::class, [
             'get' => [
                 'key' => 'csrf_key',
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class);
         $this->assertFalse($o->validate($request));
     }
-    
+
     public function testValidateRequestTokenNotFound(): void
     {
         $storage = new CsrfNullStorage();
-        $storage->set('csrf_key', [
-            'value' => 'bar',
-            'expire' => time() + 1000,
-        ]);
+        $storage->set('csrf_key', 'bar', time() + 1000);
         $config = $this->getMockInstance(Config::class, [
             'get' => [
                 'key' => 'csrf_key',
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class);
         $this->assertFalse($o->validate($request));
     }
-    
+
     public function testValidateRequestTokenNotMatch(): void
     {
         $storage = new CsrfNullStorage();
-        $storage->set('csrf_key', [
-            'value' => 'bar',
-            'expire' => time() + 1000,
-        ]);
+        $storage->set('csrf_key', 'bar', time() + 1000);
         $config = $this->getMockInstance(Config::class, [
             'get' => [
                 'key' => 'csrf_key',
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class, [
             'getParsedBody' => ['csrf_key' => 'foo']
         ]);
         $this->assertFalse($o->validate($request));
     }
-    
+
     public function testValidateNotUnique(): void
     {
         $storage = new CsrfNullStorage();
-        $storage->set('csrf_key', [
-            'value' => 'bar',
-            'expire' => time() + 1000,
-        ]);
+        $storage->set('csrf_key', 'bar', time() + 1000);
         $config = $this->getMockInstance(Config::class, [
             'get' => [
                 'key' => 'csrf_key',
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class, [
             'getParsedBody' => ['csrf_key' => 'bar']
         ]);
-        $this->assertIsArray($storage->get('csrf_key'));
         $this->assertTrue($o->validate($request));
         $this->assertNull($storage->get('csrf_key'));
     }
-    
+
     public function testValidateIsUnique(): void
     {
         $storage = new CsrfNullStorage();
-        $storage->set('csrf_key', [
-            'value' => 'bar',
-            'expire' => time() + 1000,
-        ]);
+        $storage->set('csrf_key', 'bar', time() + 1000);
         $config = $this->getMockInstance(Config::class, [
             'get' => [
                 'key' => 'csrf_key',
                 'expire' => 600,
             ]
         ]);
-        
+
         $o = new CsrfManager($config, $storage);
-        
+
         $request = $this->getMockInstance(ServerRequest::class, [
             'getParsedBody' => ['csrf_key' => 'bar']
         ]);
-        
+
         $o->unique(true);
-        
-        $this->assertIsArray($storage->get('csrf_key'));
+
+        $this->assertEquals('bar', $storage->get('csrf_key'));
         $this->assertTrue($o->validate($request));
-        $this->assertIsArray($storage->get('csrf_key'));
-        
+        $this->assertEquals('bar', $storage->get('csrf_key'));
+
         $o->clear();
         $this->assertNull($storage->get('csrf_key'));
     }
-
 }
diff --git a/tests/Security/Csrf/Storage/CsrfNullStorageTest.php b/tests/Security/Csrf/Storage/CsrfNullStorageTest.php
@@ -16,36 +16,25 @@ class CsrfNullStorageTest extends PlatineTestCase
     public function testGetNull(): void
     {
         $o = new CsrfNullStorage();
-        
+
         $this->assertNull($o->get('token'));
     }
-    
+
     public function testAll(): void
     {
         $o = new CsrfNullStorage();
-        
+
         $this->assertNull($o->get('token'));
-
-        $o->set('token', [
-            'expire' => 100,
-            'value' => 'foobar',
-        ]);
-
+
+        $o->set('token', 'foobar', 100);
+
         $res = $o->get('token');
-        $this->assertIsArray($res);
-        $this->assertCount(2, $res);
-        $this->assertArrayHasKey('expire', $res);
-        $this->assertArrayHasKey('value', $res);
-        $this->assertEquals('foobar', $res['value']);
-        $this->assertEquals(100, $res['expire']);
+        $this->assertEquals('foobar', $res);
         $o->delete('token');
-        
+
         $this->assertNull($o->get('token'));
-        
+
         $o->clear();
         $this->assertNull($o->get('token'));
     }
-
-
-
 }