refactor: get rid of $_COOKIE and $_SESSION usage in Token class (#3202)

thorsten · Nov 4, 2024 · bdf503e · bdf503e
1 parent 5347ef6
commit bdf503e
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 39 deletions.
diff --git a/phpmyfaq/src/phpMyFAQ/Session/Token.php b/phpmyfaq/src/phpMyFAQ/Session/Token.php
@@ -163,19 +163,22 @@ public function verifyToken(string $page, ?string $requestToken = null, bool $re
 
     public function removeToken(string $page): bool
     {
-        unset($_COOKIE[$this->getCookie($page)], $_SESSION[self::PMF_SESSION_NAME][$page]);
+        Request::createFromGlobals()->cookies->remove($this->getCookieName($page));
+        $this->session->remove(sprintf('%s.%s', self::PMF_SESSION_NAME, $page));
 
         return true;
     }
 
     private function getSession(string $page): ?Token
     {
-        return empty($_SESSION[self::PMF_SESSION_NAME][$page]) ? null : $_SESSION[self::PMF_SESSION_NAME][$page];
+        return $this->session->get(sprintf('%s.%s', self::PMF_SESSION_NAME, $page));
     }
 
     private function getCookie(string $page): string
     {
-        return empty($_COOKIE[$this->getCookieName($page)]) ? '' : $_COOKIE[$this->getCookieName($page)];
+        $cookieValue = Request::createFromGlobals()->cookies->get($this->getCookieName($page), '');
+
+        return empty($cookieValue) ? '' : $cookieValue;
     }
 
     /**
@@ -205,7 +208,9 @@ private function setSession(string $page, int $expiry): Token
             ]
         );
 
-        return $_SESSION[self::PMF_SESSION_NAME][$page] = $token;
+        $this->session->set(sprintf('%s.%s', self::PMF_SESSION_NAME, $page), $token);
+
+        return $token;
     }
 
     private function getCookieName(string $page): string

diff --git a/tests/phpMyFAQ/Session/TokenTest.php b/tests/phpMyFAQ/Session/TokenTest.php
@@ -8,68 +8,89 @@
 
 class TokenTest extends TestCase
 {
-    private SessionInterface $session;
+    private SessionInterface $sessionMock;
+    private Token $token;
 
     /**
      * @throws Exception
+     * @throws \Exception
      */
     protected function setUp(): void
     {
-        $_SERVER['SERVER_PORT'] = 443;
-        $this->session = $this->createMock(SessionInterface::class);
+        // Mock the SessionInterface
+        $this->sessionMock = $this->createMock(SessionInterface::class);
+        $this->token = Token::getInstance($this->sessionMock);
     }
 
-    public function testGetInstance(): void
+    public function testGetInstanceReturnsTokenInstance(): void
     {
-        $token1 = Token::getInstance($this->session);
-        $token2 = Token::getInstance($this->session);
+        $this->assertInstanceOf(Token::class, $this->token);
+    }
 
-        $this->assertInstanceOf(Token::class, $token1);
-        $this->assertSame($token1, $token2);
+    public function testSetAndGetPage(): void
+    {
+        $this->token->setPage('testPage');
+        $this->assertEquals('testPage', $this->token->getPage());
     }
 
-    /**
-     * @throws \Exception
-     */
-    public function testGetTokenInput(): void
+    public function testSetAndGetExpiry(): void
     {
-        $token = Token::getInstance($this->session);
-        $page = 'example_page';
+        $this->token->setExpiry(3600);
+        $this->assertEquals(3600, $this->token->getExpiry());
+    }
 
-        $tokenInput = $token->getTokenInput($page);
+    public function testSetAndGetSessionToken(): void
+    {
+        $this->token->setSessionToken('testSessionToken');
+        $this->assertEquals('testSessionToken', $this->token->getSessionToken());
+    }
 
-        $this->assertStringContainsString(
-            '<input type="hidden" id="pmf-csrf-token" name="pmf-csrf-token" value="',
-            $tokenInput
-        );
+    public function testSetAndGetCookieToken(): void
+    {
+        $this->token->setCookieToken('testCookieToken');
+        $this->assertEquals('testCookieToken', $this->token->getCookieToken());
+    }
+
+    public function testGetTokenInput(): void
+    {
+        $this->sessionMock
+            ->method('get')
+            ->willReturn($this->token->setSessionToken('testToken'));
+
+        $inputHtml = $this->token->getTokenInput('testPage');
+        $expectedHtml = '<input type="hidden" id="pmf-csrf-token" name="pmf-csrf-token"';
+        $this->assertStringContainsString($expectedHtml, $inputHtml);
     }
 
     /**
      * @throws \Exception
      */
-    public function testVerifyToken(): void
+    public function testGetTokenString(): void
     {
-        $token = Token::getInstance($this->session);
-        $page = 'example_page';
+        $this->sessionMock
+            ->method('get')
+            ->willReturn($this->token->setSessionToken('testToken'));
 
-        $_POST['pmf-csrf-token'] = $token->getTokenString($page);
-        $_COOKIE[sprintf('%s-%s', 'pmf-csrf-token', substr(md5($page), 0, 10))] = $token->getTokenString($page);
+        $tokenString = $this->token->getTokenString('testPage');
+        $this->assertIsString($tokenString);
+    }
 
-        $this->assertTrue($token->verifyToken($page, $_POST['pmf-csrf-token']));
+    public function testVerifyTokenReturnsFalseForInvalidToken(): void
+    {
+        $this->token->setSessionToken('testSessionToken');
+        $this->sessionMock
+            ->method('get')
+            ->willReturn($this->token);
+
+        $this->assertFalse($this->token->verifyToken('testPage', 'invalidToken'));
     }
 
-    /**
-     * @throws \Exception
-     */
     public function testRemoveToken(): void
     {
-        $token = Token::getInstance($this->session);
-        $page = 'example_page';
-
-        // Add a token to session and cookie
-        $token->getTokenString($page);
+        $this->sessionMock
+            ->method('remove')
+            ->with($this->equalTo('pmf-csrf-token.testPage'));
 
-        // Remove the token
-        $this->assertTrue($token->removeToken($page));
+        $this->assertTrue($this->token->removeToken('testPage'));
     }
 }