refactor: use Symfony's Cookie class now (#3202)

thorsten · Oct 29, 2024 · 8211edc · 8211edc
1 parent d5a4fa0
commit 8211edc
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 45 deletions.
diff --git a/phpmyfaq/index.php b/phpmyfaq/index.php
@@ -248,8 +248,8 @@
 //
 // Found a session ID in _GET or _COOKIE?
 //
-$sidGet = Filter::filterVar($request->query->get(Session::PMF_GET_KEY_NAME_SESSIONID), FILTER_VALIDATE_INT);
-$sidCookie = Filter::filterVar($request->cookies->get(Session::PMF_COOKIE_NAME_SESSIONID), FILTER_VALIDATE_INT);
+$sidGet = Filter::filterVar($request->query->get(Session::KEY_NAME_SESSION_ID), FILTER_VALIDATE_INT);
+$sidCookie = Filter::filterVar($request->cookies->get(Session::COOKIE_NAME_SESSION_ID), FILTER_VALIDATE_INT);
 $faqSession = new Session($faqConfig);
 $faqSession->setCurrentUser($user);
 
@@ -277,7 +277,7 @@
 $sids = '';
 if ($faqConfig->get('main.enableUserTracking')) {
     if ($faqSession->getCurrentSessionId() > 0) {
-        $faqSession->setCookie(Session::PMF_COOKIE_NAME_SESSIONID, $faqSession->getCurrentSessionId());
+        $faqSession->setCookie(Session::COOKIE_NAME_SESSION_ID, $faqSession->getCurrentSessionId());
         if (is_null($sidCookie)) {
             $sids = sprintf('sid=%d&amp;lang=%s&amp;', $faqSession->getCurrentSessionId(), $faqLangCode);
         }
@@ -286,13 +286,12 @@
             $sids = sprintf('sid=%d&amp;lang=%s&amp;', $sidGet, $faqLangCode);
         }
     }
-} elseif (
-    !$faqSession->setCookie(
-        Session::PMF_COOKIE_NAME_SESSIONID,
+} else {
+    $faqSession->setCookie(
+        Session::COOKIE_NAME_SESSION_ID,
         $faqSession->getCurrentSessionId(),
         $request->server->get('REQUEST_TIME') + 3600
-    )
-) {
+    );
     $sids = sprintf('lang=%s&amp;', $faqLangCode);
 }
 

diff --git a/phpmyfaq/services/azure/callback.php b/phpmyfaq/services/azure/callback.php
@@ -81,7 +81,7 @@
         $user->setTokenData([
                 'refresh_token' => $oAuth->getRefreshToken(),
                 'access_token' => $oAuth->getAccessToken(),
-                'code_verifier' => $session->get(Session::PMF_AZURE_AD_OAUTH_VERIFIER),
+                'code_verifier' => $session->get(Session::ENTRA_ID_OAUTH_VERIFIER),
                 'jwt' => $oAuth->getToken()
             ]);
         $user->setSuccess(true);

diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthEntraId.php
@@ -129,8 +129,8 @@ public function authorize(): void
     {
         $this->createOAuthChallenge();
         $this->session->setCurrentSessionKey();
-        $this->session->set(Session::PMF_AZURE_AD_OAUTH_VERIFIER, $this->oAuthVerifier);
-        $this->session->setCookie(Session::PMF_AZURE_AD_OAUTH_VERIFIER, $this->oAuthVerifier, 7200, false);
+        $this->session->set(Session::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier);
+        $this->session->setCookie(Session::ENTRA_ID_OAUTH_VERIFIER, $this->oAuthVerifier, 7200, false);
 
         $oAuthURL = sprintf(
             'https://login.microsoftonline.com/%s/oauth2/v2.0/authorize' .

diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Azure/OAuth.php b/phpmyfaq/src/phpMyFAQ/Auth/Azure/OAuth.php
@@ -66,10 +66,10 @@ public function getOAuthToken(string $code): stdClass
     {
         $url = 'https://login.microsoftonline.com/' . AAD_OAUTH_TENANTID . '/oauth2/v2.0/token';
 
-        if ($this->session->get(Session::PMF_AZURE_AD_OAUTH_VERIFIER) !== '') {
-            $codeVerifier = $this->session->get(Session::PMF_AZURE_AD_OAUTH_VERIFIER);
+        if ($this->session->get(Session::ENTRA_ID_OAUTH_VERIFIER) !== '') {
+            $codeVerifier = $this->session->get(Session::ENTRA_ID_OAUTH_VERIFIER);
         } else {
-            $codeVerifier = $this->session->getCookie(Session::PMF_AZURE_AD_OAUTH_VERIFIER);
+            $codeVerifier = $this->session->getCookie(Session::ENTRA_ID_OAUTH_VERIFIER);
         }
 
         $response = $this->client->request('POST', $url, [
@@ -118,7 +118,7 @@ public function setToken(stdClass $token): OAuth
     {
         $idToken = base64_decode(explode('.', (string) $token->id_token)[1]);
         $this->token = json_decode($idToken, null, 512, JSON_THROW_ON_ERROR);
-        $this->session->set(Session::PMF_AZURE_AD_JWT, json_encode($this->token, JSON_THROW_ON_ERROR));
+        $this->session->set(Session::ENTRA_ID_JWT, json_encode($this->token, JSON_THROW_ON_ERROR));
         return $this;
     }
 

diff --git a/phpmyfaq/src/phpMyFAQ/Session.php b/phpmyfaq/src/phpMyFAQ/Session.php
@@ -22,6 +22,7 @@
 use phpMyFAQ\User\CurrentUser;
 use Random\RandomException;
 use stdClass;
+use Symfony\Component\HttpFoundation\Cookie;
 use Symfony\Component\HttpFoundation\IpUtils;
 use Symfony\Component\HttpFoundation\Request;
 
@@ -33,22 +34,22 @@
 class Session
 {
     /** @var string Name of the "remember me" cookie */
-    final public const PMF_COOKIE_NAME_REMEMBERME = 'pmf_rememberme';
+    final public const COOKIE_NAME_REMEMBER_ME = 'pmf-remember-me';
 
     /** @var string Name of the session cookie */
-    final public const PMF_COOKIE_NAME_SESSIONID = 'pmf_sid';
+    final public const COOKIE_NAME_SESSION_ID = 'pmf-sid';
 
     /** @var string Name of the session GET parameter */
-    final public const PMF_GET_KEY_NAME_SESSIONID = 'sid';
+    final public const KEY_NAME_SESSION_ID = 'sid';
 
     /** @var string EntraID session key */
-    final public const PMF_AZURE_AD_SESSIONKEY = 'phpmyfaq_aad_sessionkey';
+    final public const ENTRA_ID_SESSION_KEY = 'pmf-entra-id-session-key';
 
     /** @var string */
-    final public const PMF_AZURE_AD_OAUTH_VERIFIER = 'phpmyfaq_azure_ad_oauth_verifier';
+    final public const ENTRA_ID_OAUTH_VERIFIER = 'pmf-entra-id-oauth-verifier';
 
     /** @var string */
-    final public const PMF_AZURE_AD_JWT = 'phpmyfaq_azure_ad_jwt';
+    final public const ENTRA_ID_JWT = 'pmf-entra-id-jwt';
 
     private ?int $currentSessionId = null;
 
@@ -92,7 +93,7 @@ public function setCurrentUser(CurrentUser $currentUser): Session
      */
     public function getCurrentSessionKey(): ?string
     {
-        return $this->currentSessionKey ?? $this->get(self::PMF_AZURE_AD_SESSIONKEY);
+        return $this->currentSessionKey ?? $this->get(self::ENTRA_ID_SESSION_KEY);
     }
 
     /**
@@ -106,7 +107,7 @@ public function setCurrentSessionKey(): Session
             $this->createCurrentSessionKey();
         }
 
-        $this->set(self::PMF_AZURE_AD_SESSIONKEY, $this->currentSessionKey);
+        $this->set(self::ENTRA_ID_SESSION_KEY, $this->currentSessionKey);
 
         return $this;
     }
@@ -289,10 +290,10 @@ public function userTracking(string $action, int|string|null $data = null): void
         $bots = 0;
         $banned = false;
         $this->currentSessionId = Filter::filterVar(
-            $request->query->get(self::PMF_GET_KEY_NAME_SESSIONID),
+            $request->query->get(self::KEY_NAME_SESSION_ID),
             FILTER_VALIDATE_INT
         );
-        $cookieId = Filter::filterVar($request->query->get(self::PMF_COOKIE_NAME_SESSIONID), FILTER_VALIDATE_INT);
+        $cookieId = Filter::filterVar($request->query->get(self::COOKIE_NAME_SESSION_ID), FILTER_VALIDATE_INT);
 
         if (!is_null($cookieId)) {
             $this->setCurrentSessionId($cookieId);
@@ -335,7 +336,7 @@ public function userTracking(string $action, int|string|null $data = null): void
                 );
                 // Check: force the session cookie to contains the current $sid
                 if (!is_null($cookieId) && (!$cookieId != $this->getCurrentSessionId())) {
-                    self::setCookie(self::PMF_COOKIE_NAME_SESSIONID, $this->getCurrentSessionId());
+                    self::setCookie(self::COOKIE_NAME_SESSION_ID, $this->getCurrentSessionId());
                 }
 
                 $query = sprintf(
@@ -381,22 +382,18 @@ public function userTracking(string $action, int|string|null $data = null): void
      * @param int|string|null $sessionId Session ID
      * @param int             $timeout Cookie timeout
      */
-    public function setCookie(string $name, int|string|null $sessionId, int $timeout = 3600, bool $strict = true): bool
+    public function setCookie(string $name, int|string|null $sessionId, int $timeout = 3600, bool $strict = true): void
     {
         $request = Request::createFromGlobals();
 
-        return setcookie(
-            $name,
-            $sessionId ?? '',
-            [
-                'expires' => $request->server->get('REQUEST_TIME') + $timeout,
-                'path' => dirname($request->server->get('SCRIPT_NAME')),
-                'domain' => parse_url($this->configuration->getDefaultUrl(), PHP_URL_HOST),
-                'samesite' => $strict ? 'strict' : '',
-                'secure' => $request->isSecure(),
-                'httponly' => true,
-            ]
-        );
+        Cookie::create($name)
+            ->withValue($sessionId ?? '')
+            ->withExpires($request->server->get('REQUEST_TIME') + $timeout)
+            ->withPath(dirname($request->server->get('SCRIPT_NAME')))
+            ->withDomain(parse_url($this->configuration->getDefaultUrl(), PHP_URL_HOST))
+            ->withSameSite($strict ? 'strict' : '')
+            ->withSecure($request->isSecure())
+            ->withHttpOnly();
     }
 
     /**

diff --git a/phpmyfaq/src/phpMyFAQ/User/CurrentUser.php b/phpmyfaq/src/phpMyFAQ/User/CurrentUser.php
@@ -172,7 +172,7 @@ public function login(string $login, string $password): bool
                 $rememberMe = sha1(session_id());
                 $this->setRememberMe($rememberMe);
                 $this->session->setCookie(
-                    Session::PMF_COOKIE_NAME_REMEMBERME,
+                    Session::COOKIE_NAME_REMEMBER_ME,
                     $rememberMe,
                     Request::createFromGlobals()->server->get('REQUEST_TIME') + self::PMF_REMEMBER_ME_EXPIRED_TIME
                 );
@@ -434,7 +434,7 @@ public function deleteFromSession(bool $deleteCookie = false): bool
         }
 
         if ($deleteCookie) {
-            $this->session->setCookie(Session::PMF_COOKIE_NAME_REMEMBERME, '');
+            $this->session->setCookie(Session::COOKIE_NAME_REMEMBER_ME, '');
         }
 
         session_destroy();
@@ -566,13 +566,13 @@ public static function getFromSession(Configuration $configuration): ?CurrentUse
     public static function getFromCookie(Configuration $configuration): ?CurrentUser
     {
         $request = Request::createFromGlobals();
-        if ($request->cookies->get(Session::PMF_COOKIE_NAME_REMEMBERME) === null) {
+        if ($request->cookies->get(Session::COOKIE_NAME_REMEMBER_ME) === null) {
             return null;
         }
 
         // create a new CurrentUser object
         $user = new self($configuration);
-        $user->getUserByCookie($request->cookies->get(Session::PMF_COOKIE_NAME_REMEMBERME));
+        $user->getUserByCookie($request->cookies->get(Session::COOKIE_NAME_REMEMBER_ME));
 
         if (-1 === $user->getUserId()) {
             return null;

diff --git a/tests/phpMyFAQ/Auth/Azure/OAuthTest.php b/tests/phpMyFAQ/Auth/Azure/OAuthTest.php
@@ -50,7 +50,7 @@ public function testGetOAuthTokenSuccess(): void
 
         $this->mockSession->expects($this->exactly(1))
             ->method('get')
-            ->with(Session::PMF_AZURE_AD_OAUTH_VERIFIER)
+            ->with(Session::ENTRA_ID_OAUTH_VERIFIER)
             ->willReturnOnConsecutiveCalls('', 'code_verifier');
 
         $this->mockClient->expects($this->once())
@@ -115,7 +115,7 @@ public function testSetToken(): void
 
         $this->mockSession->expects($this->once())
             ->method('set')
-            ->with(Session::PMF_AZURE_AD_JWT, $this->stringContains('John Doe'));
+            ->with(Session::ENTRA_ID_JWT, $this->stringContains('John Doe'));
 
         $this->oAuth->setToken($token);