Testing AntiCSRF, revealed errors

src/AntiCSRF/Base.php

@@ -4,12 +4,12 @@ namespace BradyMcD\TAATP\AntiCSRF;
 use BradyMcD\TAATP\AntiCSRFInterface;
 use BradyMcD\TAATP\SessionInterface;
 
-define("CSRF_TOKEN_IDX", "antiCSRF_TAATP");
-define("CSRF_EXPIRY_IDX", "antiCSRF_TAATP_Expiry");
-
 class Base implements AntiCSRFInterface
 {
-    private int $token;
+    const CSRF_TOKEN_IDX = "antiCSRF";
+    const CSRF_EXPIRY_IDX = "antiCSRF_Expiry";
+
+    private string $token;
 
     public function __construct(
         private SessionInterface $session,
@@ -23,7 +23,9 @@ class Base implements AntiCSRFInterface
     /** @SuppressWarnings(PHPMD.Superglobals) */
     public function match(): bool
     {
-        if (\hash_equals($this->session->get(CSRF_TOKEN_IDX), $_REQUEST[CSRF_TOKEN_IDX]))
+        $request_token = $_REQUEST[self::CSRF_TOKEN_IDX];
+
+        if (!is_null($request_token) && \hash_equals($this->token, $request_token))
         {
             return !$this->expired();
         }
@@ -32,14 +34,14 @@ class Base implements AntiCSRFInterface
 
     public function expired(): bool
     {
-        return $this->clock->now()->getTimestamp() >= $this->session->get(CSRF_EXPIRY_IDX);
+        return $this->clock->now()->getTimestamp() >= $this->session->get(self::CSRF_EXPIRY_IDX);
     }
 
     public function emitStr(): string
     {
         return
             '<input type="hidden" name="'
-            . CSRF_TOKEN_IDX
+            . self::CSRF_TOKEN_IDX
             . '" value="'
             . $this->token
             . '" />';
@@ -57,16 +59,16 @@ class Base implements AntiCSRFInterface
 
     public function generate()
     {
-        $this->session->tryStore(CSRF_TOKEN_IDX, \bin2hex(\random_bytes(32)));
+        $this->session->tryStore(self::CSRF_TOKEN_IDX, \bin2hex(\random_bytes(32)));
-        $this->token = $this->session->get(CSRF_TOKEN_IDX);
+        $this->token = $this->session->get(self::CSRF_TOKEN_IDX);
-        $this->session->tryStore(CSRF_EXPIRY_IDX, $this->expiry());
+        $this->session->tryStore(self::CSRF_EXPIRY_IDX, $this->expiry());
     }
 
     public function regenerate()
     {
-        $this->session->store(CSRF_TOKEN_IDX, \bin2hex(\random_bytes(32)));
+        $this->session->store(self::CSRF_TOKEN_IDX, \bin2hex(\random_bytes(32)));
-        $this->token = $this->session->get(CSRF_TOKEN_IDX);
+        $this->token = $this->session->get(self::CSRF_TOKEN_IDX);
-        $this->session->store(CSRF_EXPIRY_IDX, $this->expiry());
+        $this->session->store(self::CSRF_EXPIRY_IDX, $this->expiry());
     }
 }
 

src/AntiCSRFInterface.php

@@ -3,12 +3,13 @@
 namespace BradyMcD\TAATP;
 
 /**
- * 
+ *
  */
 interface AntiCSRFInterface
 {
     public function __construct(
-        SessionInterface $session
+        SessionInterface $session,
+        \Psr\Clock\ClockInterface $clock
     );
     public function match(): bool;
     public function expired(): bool;

src/SessionInterface.php

@@ -11,7 +11,7 @@ interface SessionInterface
     function live(): bool;
     function tryStore(string $key, mixed $val): bool;
     function store(string $key, mixed $val);
-    function get(string $key);
+    function get(string $key): mixed;
 }
 
 ?>

tests/AntiCSRFTest.php

@@ -0,0 +1,81 @@
+<?php declare(strict_types=1);
+
+use PHPUnit\Framework\TestCase;
+
+use BradyMcD\TAATP\AntiCSRF\Base as BaseAntiCSRF;
+use BradyMcD\TAATP\Session\Base as BaseSession;
+
+final class TestClock implements \Psr\Clock\ClockInterface
+{
+    private static $time;
+
+    public function __construct()
+    {
+        self::$time = 42;
+    }
+
+    function setTime(int $t): void
+    {
+        self::$time = $t;
+    }
+
+    function now(): DateTimeImmutable
+    {
+        return (new DateTimeImmutable())->setTimestamp(self::$time);
+    }
+}
+
+/** @SuppressWarnings(PHPMD.StaticAccess)*/
+final class AntiCSRFTest extends TestCase
+{
+    private static $clock;
+    private static $AntiCSRF;
+    private static $session;
+
+    public static function setUpBeforeClass(): void
+    {
+        self::$session = new BaseSession();
+        self::$clock = new TestClock();
+        self::$AntiCSRF = new BaseAntiCSRF(self::$session, self::$clock);
+    }
+
+    public function testRAIITokenGeneration(): void
+    {
+        $this->assertIsString(self::$session->get(BaseAntiCSRF::CSRF_TOKEN_IDX));
+        $this->assertIsInt(self::$session->get(BaseAntiCSRF::CSRF_EXPIRY_IDX));
+    }
+
+    public function testTokenRegeneration(): void
+    {
+        $currToken = self::$session->get(BaseAntiCSRF::CSRF_TOKEN_IDX);
+        self::$AntiCSRF->regenerate();
+        $this->assertNotEquals($currToken, self::$session->get(BaseAntiCSRF::CSRF_TOKEN_IDX));
+    }
+
+    public function testMatchRejectsMissingToken(): void
+    {
+        $this->assertFalse(self::$AntiCSRF->match());
+    }
+
+    public function testMatchRejectsWrongToken(): void
+    {
+        $_REQUEST[BaseAntiCSRF::CSRF_TOKEN_IDX] = "Not a token";
+
+        $this->assertFalse(self::$AntiCSRF->match());
+    }
+
+    public function testMatchAcceptsToken(): void
+    {
+        $_REQUEST[BaseAntiCSRF::CSRF_TOKEN_IDX] = self::$session->get(BaseAntiCSRF::CSRF_TOKEN_IDX);
+
+        $this->assertTrue(self::$AntiCSRF->match());
+    }
+
+    public function testMatchRejectsExpired(): void
+    {
+        self::$clock->setTime(self::$clock->now()->getTimestamp() + 3600);
+
+        $this->assertFalse(self::$AntiCSRF->match());
+    }
+
+}