Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove Cookie config items in Config\App #7221

Merged
merged 9 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 0 additions & 79 deletions app/Config/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions app/Config/Cookie.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
29 changes: 5 additions & 24 deletions system/HTTP/Response.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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');
Expand Down
6 changes: 2 additions & 4 deletions system/Helpers/cookie_helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
* the LICENSE file that was distributed with this source code.
*/

use Config\App;
use Config\Cookie;
use Config\Services;

Expand Down Expand Up @@ -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();
Expand Down
22 changes: 8 additions & 14 deletions system/Security/Security.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
}

/**
Expand Down
19 changes: 5 additions & 14 deletions system/Session/Handlers/BaseHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
14 changes: 4 additions & 10 deletions system/Session/Session.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 0 additions & 6 deletions system/Test/Mock/MockAppConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
6 changes: 0 additions & 6 deletions system/Test/Mock/MockCLIConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
42 changes: 30 additions & 12 deletions tests/system/API/ResponseTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace CodeIgniter\API;

use CodeIgniter\Config\Factories;
use CodeIgniter\Format\FormatterInterface;
use CodeIgniter\Format\JSONFormatter;
use CodeIgniter\Format\XMLFormatter;
Expand All @@ -20,6 +21,7 @@
use CodeIgniter\Test\Mock\MockIncomingRequest;
use CodeIgniter\Test\Mock\MockResponse;
use Config\App;
use Config\Cookie;
use stdClass;

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
Loading