Merge pull request #7221 from kenjis/remove-config-app-cookie-4.4

refactor: remove Cookie config items in Config\App
codeigniter4 · Feb 7, 2023 · f8ae5a1 · f8ae5a1
2 parents 7ea9372 + 01f7a6b
commit f8ae5a1
Show file tree

Hide file tree

Showing 17 changed files with 168 additions and 202 deletions.
diff --git a/app/Config/App.php b/app/Config/App.php
@@ -238,85 +238,6 @@ class App extends BaseConfig
      */
     public ?string $sessionDBGroup = null;
 
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie Prefix
-     * --------------------------------------------------------------------------
-     *
-     * Set a cookie name prefix if you need to avoid collisions.
-     *
-     * @deprecated use Config\Cookie::$prefix property instead.
-     */
-    public string $cookiePrefix = '';
-
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie Domain
-     * --------------------------------------------------------------------------
-     *
-     * Set to `.your-domain.com` for site-wide cookies.
-     *
-     * @deprecated use Config\Cookie::$domain property instead.
-     */
-    public string $cookieDomain = '';
-
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie Path
-     * --------------------------------------------------------------------------
-     *
-     * Typically will be a forward slash.
-     *
-     * @deprecated use Config\Cookie::$path property instead.
-     */
-    public string $cookiePath = '/';
-
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie Secure
-     * --------------------------------------------------------------------------
-     *
-     * Cookie will only be set if a secure HTTPS connection exists.
-     *
-     * @deprecated use Config\Cookie::$secure property instead.
-     */
-    public bool $cookieSecure = false;
-
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie HttpOnly
-     * --------------------------------------------------------------------------
-     *
-     * Cookie will only be accessible via HTTP(S) (no JavaScript).
-     *
-     * @deprecated use Config\Cookie::$httponly property instead.
-     */
-    public bool $cookieHTTPOnly = true;
-
-    /**
-     * --------------------------------------------------------------------------
-     * Cookie SameSite
-     * --------------------------------------------------------------------------
-     *
-     * Configure cookie SameSite setting. Allowed values are:
-     * - None
-     * - Lax
-     * - Strict
-     * - ''
-     *
-     * Alternatively, you can use the constant names:
-     * - `Cookie::SAMESITE_NONE`
-     * - `Cookie::SAMESITE_LAX`
-     * - `Cookie::SAMESITE_STRICT`
-     *
-     * Defaults to `Lax` for compatibility with modern browsers. Setting `''`
-     * (empty string) means default SameSite attribute set by browsers (`Lax`)
-     * will be set on cookies. If set to `None`, `$cookieSecure` must also be set.
-     *
-     * @deprecated use Config\Cookie::$samesite property instead.
-     */
-    public ?string $cookieSameSite = 'Lax';
-
     /**
      * --------------------------------------------------------------------------
      * Reverse Proxy IPs

diff --git a/app/Config/Cookie.php b/app/Config/Cookie.php
@@ -84,6 +84,8 @@ class Cookie extends BaseConfig
      * Defaults to `Lax` for compatibility with modern browsers. Setting `''`
      * (empty string) means default SameSite attribute set by browsers (`Lax`)
      * will be set on cookies. If set to `None`, `$secure` must also be set.
+     *
+     * @phpstan-var 'None'|'Lax'|'Strict'|''
      */
     public string $samesite = 'Lax';
 

diff --git a/system/HTTP/Response.php b/system/HTTP/Response.php
@@ -13,9 +13,9 @@
 
 use CodeIgniter\Cookie\Cookie;
 use CodeIgniter\Cookie\CookieStore;
-use CodeIgniter\Cookie\Exceptions\CookieException;
 use CodeIgniter\HTTP\Exceptions\HTTPException;
 use Config\App;
+use Config\Cookie as CookieConfig;
 use Config\Services;
 
 /**
@@ -156,31 +156,12 @@ public function __construct($config)
 
         $this->CSPEnabled = $config->CSPEnabled;
 
-        // DEPRECATED COOKIE MANAGEMENT
-
-        $this->cookiePrefix   = $config->cookiePrefix;
-        $this->cookieDomain   = $config->cookieDomain;
-        $this->cookiePath     = $config->cookiePath;
-        $this->cookieSecure   = $config->cookieSecure;
-        $this->cookieHTTPOnly = $config->cookieHTTPOnly;
-        $this->cookieSameSite = $config->cookieSameSite ?? Cookie::SAMESITE_LAX;
+        $this->cookieStore = new CookieStore([]);
 
-        $config->cookieSameSite ??= Cookie::SAMESITE_LAX;
+        /** @var CookieConfig $cookie */
+        $cookie = config('Cookie');
 
-        if (! in_array(strtolower($config->cookieSameSite ?: Cookie::SAMESITE_LAX), Cookie::ALLOWED_SAMESITE_VALUES, true)) {
-            throw CookieException::forInvalidSameSite($config->cookieSameSite);
-        }
-
-        $this->cookieStore = new CookieStore([]);
-        Cookie::setDefaults(config('Cookie') ?? [
-            // @todo Remove this fallback when deprecated `App` members are removed
-            'prefix'   => $config->cookiePrefix,
-            'path'     => $config->cookiePath,
-            'domain'   => $config->cookieDomain,
-            'secure'   => $config->cookieSecure,
-            'httponly' => $config->cookieHTTPOnly,
-            'samesite' => $config->cookieSameSite ?? Cookie::SAMESITE_LAX,
-        ]);
+        Cookie::setDefaults($cookie);
 
         // Default to an HTML Content-Type. Devs can override if needed.
         $this->setContentType('text/html');

diff --git a/system/Helpers/cookie_helper.php b/system/Helpers/cookie_helper.php
@@ -9,7 +9,6 @@
  * the LICENSE file that was distributed with this source code.
  */
 
-use Config\App;
 use Config\Cookie;
 use Config\Services;
 
@@ -68,11 +67,10 @@ function set_cookie(
     function get_cookie($index, bool $xssClean = false, ?string $prefix = '')
     {
         if ($prefix === '') {
-            /** @var Cookie|null $cookie */
+            /** @var Cookie $cookie */
             $cookie = config('Cookie');
 
-            // @TODO Remove Config\App fallback when deprecated `App` members are removed.
-            $prefix = $cookie instanceof Cookie ? $cookie->prefix : config('App')->cookiePrefix;
+            $prefix = $cookie->prefix;
         }
 
         $request = Services::request();

diff --git a/system/Security/Security.php b/system/Security/Security.php
@@ -195,7 +195,10 @@ public function __construct(App $config)
         }
 
         if ($this->isCSRFCookie()) {
-            $this->configureCookie($config);
+            /** @var CookieConfig $cookie */
+            $cookie = config('Cookie');
+
+            $this->configureCookie($cookie);
         } else {
             // Session based CSRF protection
             $this->configureSession();
@@ -220,20 +223,11 @@ private function configureSession(): void
         $this->session = Services::session();
     }
 
-    private function configureCookie(App $config): void
+    private function configureCookie(CookieConfig $cookie): void
     {
-        /** @var CookieConfig|null $cookie */
-        $cookie = config('Cookie');
-
-        if ($cookie instanceof CookieConfig) {
-            $cookiePrefix     = $cookie->prefix;
-            $this->cookieName = $cookiePrefix . $this->rawCookieName;
-            Cookie::setDefaults($cookie);
-        } else {
-            // `Config/Cookie.php` is absence
-            $cookiePrefix     = $config->cookiePrefix;
-            $this->cookieName = $cookiePrefix . $this->rawCookieName;
-        }
+        $cookiePrefix     = $cookie->prefix;
+        $this->cookieName = $cookiePrefix . $this->rawCookieName;
+        Cookie::setDefaults($cookie);
     }
 
     /**

diff --git a/system/Session/Handlers/BaseHandler.php b/system/Session/Handlers/BaseHandler.php
@@ -122,22 +122,13 @@ public function __construct(AppConfig $config, string $ipAddress)
             $this->savePath   = $config->sessionSavePath;
         }
 
-        /** @var CookieConfig|null $cookie */
+        /** @var CookieConfig $cookie */
         $cookie = config('Cookie');
 
-        if ($cookie instanceof CookieConfig) {
-            // Session cookies have no prefix.
-            $this->cookieDomain = $cookie->domain;
-            $this->cookiePath   = $cookie->path;
-            $this->cookieSecure = $cookie->secure;
-        } else {
-            // @TODO Remove this fallback when deprecated `App` members are removed.
-            // `Config/Cookie.php` is absence
-            // Session cookies have no prefix.
-            $this->cookieDomain = $config->cookieDomain;
-            $this->cookiePath   = $config->cookiePath;
-            $this->cookieSecure = $config->cookieSecure;
-        }
+        // Session cookies have no prefix.
+        $this->cookieDomain = $cookie->domain;
+        $this->cookiePath   = $cookie->path;
+        $this->cookieSecure = $cookie->secure;
 
         $this->ipAddress = $ipAddress;
     }

diff --git a/system/Session/Session.php b/system/Session/Session.php
@@ -188,22 +188,16 @@ public function __construct(SessionHandlerInterface $driver, App $config)
             $this->sessionRegenerateDestroy = $config->sessionRegenerateDestroy ?? $this->sessionRegenerateDestroy;
         }
 
-        // DEPRECATED COOKIE MANAGEMENT
-        $this->cookiePath     = $config->cookiePath ?? $this->cookiePath;
-        $this->cookieDomain   = $config->cookieDomain ?? $this->cookieDomain;
-        $this->cookieSecure   = $config->cookieSecure ?? $this->cookieSecure;
-        $this->cookieSameSite = $config->cookieSameSite ?? $this->cookieSameSite;
-
         /** @var CookieConfig $cookie */
         $cookie = config('Cookie');
 
         $this->cookie = (new Cookie($this->sessionCookieName, '', [
             'expires'  => $this->sessionExpiration === 0 ? 0 : Time::now()->getTimestamp() + $this->sessionExpiration,
-            'path'     => $cookie->path ?? $config->cookiePath,
-            'domain'   => $cookie->domain ?? $config->cookieDomain,
-            'secure'   => $cookie->secure ?? $config->cookieSecure,
+            'path'     => $cookie->path,
+            'domain'   => $cookie->domain,
+            'secure'   => $cookie->secure,
             'httponly' => true, // for security
-            'samesite' => $cookie->samesite ?? $config->cookieSameSite ?? Cookie::SAMESITE_LAX,
+            'samesite' => $cookie->samesite ?? Cookie::SAMESITE_LAX,
             'raw'      => $cookie->raw ?? false,
         ]))->withPrefix(''); // Cookie prefix should be ignored.
 

diff --git a/system/Test/Mock/MockAppConfig.php b/system/Test/Mock/MockAppConfig.php
@@ -17,12 +17,6 @@ class MockAppConfig extends App
 {
     public string $baseURL         = 'http://example.com/';
     public string $uriProtocol     = 'REQUEST_URI';
-    public string $cookiePrefix    = '';
-    public string $cookieDomain    = '';
-    public string $cookiePath      = '/';
-    public bool $cookieSecure      = false;
-    public bool $cookieHTTPOnly    = false;
-    public ?string $cookieSameSite = 'Lax';
     public array $proxyIPs         = [];
     public string $CSRFTokenName   = 'csrf_test_name';
     public string $CSRFHeaderName  = 'X-CSRF-TOKEN';

diff --git a/system/Test/Mock/MockCLIConfig.php b/system/Test/Mock/MockCLIConfig.php
@@ -17,12 +17,6 @@ class MockCLIConfig extends App
 {
     public string $baseURL         = 'http://example.com/';
     public string $uriProtocol     = 'REQUEST_URI';
-    public string $cookiePrefix    = '';
-    public string $cookieDomain    = '';
-    public string $cookiePath      = '/';
-    public bool $cookieSecure      = false;
-    public bool $cookieHTTPOnly    = false;
-    public ?string $cookieSameSite = 'Lax';
     public array $proxyIPs         = [];
     public string $CSRFTokenName   = 'csrf_test_name';
     public string $CSRFCookieName  = 'csrf_cookie_name';

diff --git a/tests/system/API/ResponseTraitTest.php b/tests/system/API/ResponseTraitTest.php
@@ -11,6 +11,7 @@
 
 namespace CodeIgniter\API;
 
+use CodeIgniter\Config\Factories;
 use CodeIgniter\Format\FormatterInterface;
 use CodeIgniter\Format\JSONFormatter;
 use CodeIgniter\Format\XMLFormatter;
@@ -20,6 +21,7 @@
 use CodeIgniter\Test\Mock\MockIncomingRequest;
 use CodeIgniter\Test\Mock\MockResponse;
 use Config\App;
+use Config\Cookie;
 use stdClass;
 
 /**
@@ -59,17 +61,25 @@ protected function makeController(array $userConfig = [], string $uri = 'http://
             'negotiateLocale'  => false,
             'supportedLocales' => ['en'],
             'CSPEnabled'       => false,
-            'cookiePrefix'     => '',
-            'cookieDomain'     => '',
-            'cookiePath'       => '/',
-            'cookieSecure'     => false,
-            'cookieHTTPOnly'   => false,
             'proxyIPs'         => [],
-            'cookieSameSite'   => 'Lax',
         ] as $key => $value) {
             $config->{$key} = $value;
         }
 
+        $cookie = new Cookie();
+
+        foreach ([
+            'prefix'   => '',
+            'domain'   => '',
+            'path'     => '/',
+            'secure'   => false,
+            'httponly' => false,
+            'samesite' => 'Lax',
+        ] as $key => $value) {
+            $cookie->{$key} = $value;
+        }
+        Factories::injectMock('config', 'Cookie', $cookie);
+
         if ($this->request === null) {
             $this->request  = new MockIncomingRequest($config, new URI($uri), null, new UserAgent());
             $this->response = new MockResponse($config);
@@ -532,17 +542,25 @@ public function testFormatByRequestNegotiateIfFormatIsNotJsonOrXML()
             'negotiateLocale'  => false,
             'supportedLocales' => ['en'],
             'CSPEnabled'       => false,
-            'cookiePrefix'     => '',
-            'cookieDomain'     => '',
-            'cookiePath'       => '/',
-            'cookieSecure'     => false,
-            'cookieHTTPOnly'   => false,
             'proxyIPs'         => [],
-            'cookieSameSite'   => 'Lax',
         ] as $key => $value) {
             $config->{$key} = $value;
         }
 
+        $cookie = new Cookie();
+
+        foreach ([
+            'prefix'   => '',
+            'domain'   => '',
+            'path'     => '/',
+            'secure'   => false,
+            'httponly' => false,
+            'samesite' => 'Lax',
+        ] as $key => $value) {
+            $cookie->{$key} = $value;
+        }
+        Factories::injectMock('config', 'Cookie', $cookie);
+
         $request  = new MockIncomingRequest($config, new URI($config->baseURL), null, new UserAgent());
         $response = new MockResponse($config);