Skip to content

Commit

Permalink
Merge pull request #7630 from kenjis/remove-config-app-CSRF-items-4.4
Browse files Browse the repository at this point in the history 
Remove Config\App Security items
  • Loading branch information
@kenjis
kenjis authored Jun 29, 2023
2 parents 1df1034 + 24e9cad commit d9c7f06
Show file tree
Hide file tree
Showing 14 changed files with 176 additions and 203 deletions.
85 changes: 0 additions & 85 deletions app/Config/App.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,91 +158,6 @@ class App extends BaseConfig
*/
public array $proxyIPs = [];

/**
* --------------------------------------------------------------------------
* CSRF Token Name
* --------------------------------------------------------------------------
*
* The token name.
*
* @deprecated Use `Config\Security` $tokenName property instead of using this property.
*/
public string $CSRFTokenName = 'csrf_test_name';

/**
* --------------------------------------------------------------------------
* CSRF Header Name
* --------------------------------------------------------------------------
*
* The header name.
*
* @deprecated Use `Config\Security` $headerName property instead of using this property.
*/
public string $CSRFHeaderName = 'X-CSRF-TOKEN';

/**
* --------------------------------------------------------------------------
* CSRF Cookie Name
* --------------------------------------------------------------------------
*
* The cookie name.
*
* @deprecated Use `Config\Security` $cookieName property instead of using this property.
*/
public string $CSRFCookieName = 'csrf_cookie_name';

/**
* --------------------------------------------------------------------------
* CSRF Expire
* --------------------------------------------------------------------------
*
* The number in seconds the token should expire.
*
* @deprecated Use `Config\Security` $expire property instead of using this property.
*/
public int $CSRFExpire = 7200;

/**
* --------------------------------------------------------------------------
* CSRF Regenerate
* --------------------------------------------------------------------------
*
* Regenerate token on every submission?
*
* @deprecated Use `Config\Security` $regenerate property instead of using this property.
*/
public bool $CSRFRegenerate = true;

/**
* --------------------------------------------------------------------------
* CSRF Redirect
* --------------------------------------------------------------------------
*
* Redirect to previous page with error on failure?
*
* @deprecated Use `Config\Security` $redirect property instead of using this property.
*/
public bool $CSRFRedirect = false;

/**
* --------------------------------------------------------------------------
* CSRF SameSite
* --------------------------------------------------------------------------
*
* Setting for CSRF SameSite cookie token. Allowed values are:
* - None
* - Lax
* - Strict
* - ''
*
* Defaults to `Lax` as recommended in this link:
*
* @see https://portswigger.net/web-security/csrf/samesite-cookies
*
* @deprecated `Config\Cookie` $samesite property is used.
*/
public string $CSRFSameSite = 'Lax';

/**
* --------------------------------------------------------------------------
* Content Security Policy
Expand Down
5 changes: 3 additions & 2 deletions system/Config/Services.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
use Config\Pager as PagerConfig;
use Config\Paths;
use Config\Routing;
use Config\Security as SecurityConfig;
use Config\Services as AppServices;
use Config\Session as SessionConfig;
use Config\Toolbar as ToolbarConfig;
Expand Down Expand Up @@ -627,13 +628,13 @@ public static function router(?RouteCollectionInterface $routes = null, ?Request
*
* @return Security
*/
public static function security(?App $config = null, bool $getShared = true)
public static function security(?SecurityConfig $config = null, bool $getShared = true)
{
if ($getShared) {
return static::getSharedInstance('security', $config);
}

$config ??= config(App::class);
$config ??= config(SecurityConfig::class);

return new Security($config);
}
Expand Down
86 changes: 43 additions & 43 deletions system/Security/Security.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
use CodeIgniter\I18n\Time;
use CodeIgniter\Security\Exceptions\SecurityException;
use CodeIgniter\Session\Session;
use Config\App;
use Config\Cookie as CookieConfig;
use Config\Security as SecurityConfig;
use Config\Services;
Expand All @@ -44,13 +43,17 @@ class Security implements SecurityInterface
* Protection Method for Cross Site Request Forgery protection.
*
* @var string 'cookie' or 'session'
*
* @deprecated 4.4.0 Use $this->config->csrfProtection.
*/
protected $csrfProtection = self::CSRF_PROTECTION_COOKIE;

/**
* CSRF Token Randomization
*
* @var bool
*
* @deprecated 4.4.0 Use $this->config->tokenRandomize.
*/
protected $tokenRandomize = false;

Expand All @@ -69,6 +72,8 @@ class Security implements SecurityInterface
* Token name for Cross Site Request Forgery protection.
*
* @var string
*
* @deprecated 4.4.0 Use $this->config->tokenName.
*/
protected $tokenName = 'csrf_token_name';

Expand All @@ -78,6 +83,8 @@ class Security implements SecurityInterface
* Header name for Cross Site Request Forgery protection.
*
* @var string
*
* @deprecated 4.4.0 Use $this->config->headerName.
*/
protected $headerName = 'X-CSRF-TOKEN';

Expand Down Expand Up @@ -105,6 +112,8 @@ class Security implements SecurityInterface
* Defaults to two hours (in seconds).
*
* @var int
*
* @deprecated 4.4.0 Use $this->config->expires.
*/
protected $expires = 7200;

Expand All @@ -114,6 +123,8 @@ class Security implements SecurityInterface
* Regenerate CSRF Token on every request.
*
* @var bool
*
* @deprecated 4.4.0 Use $this->config->regenerate.
*/
protected $regenerate = true;

Expand All @@ -123,6 +134,8 @@ class Security implements SecurityInterface
* Redirect to previous page with error on failure.
*
* @var bool
*
* @deprecated 4.4.0 Use $this->config->redirect.
*/
protected $redirect = false;

Expand Down Expand Up @@ -163,35 +176,22 @@ class Security implements SecurityInterface
*/
private ?string $hashInCookie = null;

/**
* Security Config
*/
protected SecurityConfig $config;

/**
* Constructor.
*
* Stores our configuration and fires off the init() method to setup
* initial state.
*/
public function __construct(App $config)
public function __construct(SecurityConfig $config)
{
$security = config(SecurityConfig::class);

// Store CSRF-related configurations
if ($security instanceof SecurityConfig) {
$this->csrfProtection = $security->csrfProtection ?? $this->csrfProtection;
$this->tokenName = $security->tokenName ?? $this->tokenName;
$this->headerName = $security->headerName ?? $this->headerName;
$this->regenerate = $security->regenerate ?? $this->regenerate;
$this->redirect = $security->redirect ?? $this->redirect;
$this->rawCookieName = $security->cookieName ?? $this->rawCookieName;
$this->expires = $security->expires ?? $this->expires;
$this->tokenRandomize = $security->tokenRandomize ?? $this->tokenRandomize;
} else {
// `Config/Security.php` is absence
$this->tokenName = $config->CSRFTokenName ?? $this->tokenName;
$this->headerName = $config->CSRFHeaderName ?? $this->headerName;
$this->regenerate = $config->CSRFRegenerate ?? $this->regenerate;
$this->rawCookieName = $config->CSRFCookieName ?? $this->rawCookieName;
$this->expires = $config->CSRFExpire ?? $this->expires;
$this->redirect = $config->CSRFRedirect ?? $this->redirect;
}
$this->config = $config;

$this->rawCookieName = $config->cookieName;

if ($this->isCSRFCookie()) {
$cookie = config(CookieConfig::class);
Expand All @@ -213,7 +213,7 @@ public function __construct(App $config)

private function isCSRFCookie(): bool
{
return $this->csrfProtection === self::CSRF_PROTECTION_COOKIE;
return $this->config->csrfProtection === self::CSRF_PROTECTION_COOKIE;
}

private function configureSession(): void
Expand Down Expand Up @@ -287,7 +287,7 @@ public function verify(RequestInterface $request)
$postedToken = $this->getPostedToken($request);

try {
$token = ($postedToken !== null && $this->tokenRandomize)
$token = ($postedToken !== null && $this->config->tokenRandomize)
? $this->derandomize($postedToken) : $postedToken;
} catch (InvalidArgumentException $e) {
$token = null;
Expand All @@ -300,7 +300,7 @@ public function verify(RequestInterface $request)

$this->removeTokenInRequest($request);

if ($this->regenerate) {
if ($this->config->regenerate) {
$this->generateHash();
}

Expand All @@ -318,13 +318,13 @@ private function removeTokenInRequest(RequestInterface $request): void

$json = json_decode($request->getBody() ?? '');

if (isset($_POST[$this->tokenName])) {
if (isset($_POST[$this->config->tokenName])) {
// We kill this since we're done and we don't want to pollute the POST array.
unset($_POST[$this->tokenName]);
unset($_POST[$this->config->tokenName]);
$request->setGlobal('post', $_POST);
} elseif (isset($json->{$this->tokenName})) {
} elseif (isset($json->{$this->config->tokenName})) {
// We kill this since we're done and we don't want to pollute the JSON data.
unset($json->{$this->tokenName});
unset($json->{$this->config->tokenName});
$request->setBody(json_encode($json));
}
}
Expand All @@ -335,19 +335,19 @@ private function getPostedToken(RequestInterface $request): ?string

// Does the token exist in POST, HEADER or optionally php:://input - json data.

if ($tokenValue = $request->getPost($this->tokenName)) {
if ($tokenValue = $request->getPost($this->config->tokenName)) {
return $tokenValue;
}

if ($request->hasHeader($this->headerName) && ! empty($request->header($this->headerName)->getValue())) {
return $request->header($this->headerName)->getValue();
if ($request->hasHeader($this->config->headerName) && ! empty($request->header($this->config->headerName)->getValue())) {
return $request->header($this->config->headerName)->getValue();
}

$body = (string) $request->getBody();
$json = json_decode($body);

if ($body !== '' && ! empty($json) && json_last_error() === JSON_ERROR_NONE) {
return $json->{$this->tokenName} ?? null;
return $json->{$this->config->tokenName} ?? null;
}

return null;
Expand All @@ -358,7 +358,7 @@ private function getPostedToken(RequestInterface $request): ?string
*/
public function getHash(): ?string
{
return $this->tokenRandomize ? $this->randomize($this->hash) : $this->hash;
return $this->config->tokenRandomize ? $this->randomize($this->hash) : $this->hash;
}

/**
Expand Down Expand Up @@ -407,23 +407,23 @@ protected function derandomize(string $token): string
*/
public function getTokenName(): string
{
return $this->tokenName;
return $this->config->tokenName;
}

/**
* Returns the CSRF Header Name.
*/
public function getHeaderName(): string
{
return $this->headerName;
return $this->config->headerName;
}

/**
* Returns the CSRF Cookie Name.
*/
public function getCookieName(): string
{
return $this->cookieName;
return $this->config->cookieName;
}

/**
Expand All @@ -443,7 +443,7 @@ public function isExpired(): bool
*/
public function shouldRedirect(): bool
{
return $this->redirect;
return $this->config->redirect;
}

/**
Expand Down Expand Up @@ -521,9 +521,9 @@ private function restoreHash(): void
if ($this->isHashInCookie()) {
$this->hash = $this->hashInCookie;
}
} elseif ($this->session->has($this->tokenName)) {
} elseif ($this->session->has($this->config->tokenName)) {
// Session based CSRF protection
$this->hash = $this->session->get($this->tokenName);
$this->hash = $this->session->get($this->config->tokenName);
}
}

Expand Down Expand Up @@ -562,7 +562,7 @@ private function saveHashInCookie(): void
$this->rawCookieName,
$this->hash,
[
'expires' => $this->expires === 0 ? 0 : Time::now()->getTimestamp() + $this->expires,
'expires' => $this->config->expires === 0 ? 0 : Time::now()->getTimestamp() + $this->config->expires,
]
);

Expand Down Expand Up @@ -606,6 +606,6 @@ protected function doSendCookie(): void

private function saveHashInSession(): void
{
$this->session->set($this->tokenName, $this->hash);
$this->session->set($this->config->tokenName, $this->hash);
}
}
8 changes: 0 additions & 8 deletions system/Test/Mock/MockAppConfig.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,6 @@ class MockAppConfig extends App
public string $baseURL = 'http://example.com/';
public string $uriProtocol = 'REQUEST_URI';
public array $proxyIPs = [];
public string $CSRFTokenName = 'csrf_test_name';
public string $CSRFHeaderName = 'X-CSRF-TOKEN';
public string $CSRFCookieName = 'csrf_cookie_name';
public int $CSRFExpire = 7200;
public bool $CSRFRegenerate = true;
public array $CSRFExcludeURIs = ['http://example.com'];
public bool $CSRFRedirect = false;
public string $CSRFSameSite = 'Lax';
public bool $CSPEnabled = false;
public string $defaultLocale = 'en';
public bool $negotiateLocale = false;
Expand Down
Loading

0 comments on commit d9c7f06

Please sign in to comment.