Skip to content

Commit

Permalink
Merge pull request #3961 from mostafakhudair/refactor-security-class
Browse files Browse the repository at this point in the history
Refactor Security Class
  • Loading branch information
MGatner authored Dec 8, 2020
2 parents d9e211e + 223ec5a commit a2de1f6
Show file tree
Hide file tree
Showing 14 changed files with 490 additions and 311 deletions.
14 changes: 14 additions & 0 deletions app/Config/App.php
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,8 @@ class App extends BaseConfig
*
* The token name.
*
* @deprecated Use `Config\Security` $tokenName property instead of using this property.
*
* @var string
*/
public $CSRFTokenName = 'csrf_test_name';
Expand All @@ -345,6 +347,8 @@ class App extends BaseConfig
*
* The header name.
*
* @deprecated Use `Config\Security` $headerName property instead of using this property.
*
* @var string
*/
public $CSRFHeaderName = 'X-CSRF-TOKEN';
Expand All @@ -356,6 +360,8 @@ class App extends BaseConfig
*
* The cookie name.
*
* @deprecated Use `Config\Security` $cookieName property instead of using this property.
*
* @var string
*/
public $CSRFCookieName = 'csrf_cookie_name';
Expand All @@ -367,6 +373,8 @@ class App extends BaseConfig
*
* The number in seconds the token should expire.
*
* @deprecated Use `Config\Security` $expire property instead of using this property.
*
* @var integer
*/
public $CSRFExpire = 7200;
Expand All @@ -378,6 +386,8 @@ class App extends BaseConfig
*
* Regenerate token on every submission?
*
* @deprecated Use `Config\Security` $regenerate property instead of using this property.
*
* @var boolean
*/
public $CSRFRegenerate = true;
Expand All @@ -389,6 +399,8 @@ class App extends BaseConfig
*
* Redirect to previous page with error on failure?
*
* @deprecated Use `Config\Security` $redirect property instead of using this property.
*
* @var boolean
*/
public $CSRFRedirect = true;
Expand All @@ -408,6 +420,8 @@ class App extends BaseConfig
*
* @see https://portswigger.net/web-security/csrf/samesite-cookies
*
* @deprecated Use `Config\Security` $samesite property instead of using this property.
*
* @var string
*/
public $CSRFSameSite = 'Lax';
Expand Down
92 changes: 92 additions & 0 deletions app/Config/Security.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

namespace Config;

use CodeIgniter\Config\BaseConfig;

class Security extends BaseConfig
{
/**
* --------------------------------------------------------------------------
* CSRF Token Name
* --------------------------------------------------------------------------
*
* Token name for Cross Site Request Forgery protection cookie.
*
* @var string
*/
public $tokenName = 'csrf_test_name';

/**
* --------------------------------------------------------------------------
* CSRF Header Name
* --------------------------------------------------------------------------
*
* Token name for Cross Site Request Forgery protection cookie.
*
* @var string
*/
public $headerName = 'X-CSRF-TOKEN';

/**
* --------------------------------------------------------------------------
* CSRF Cookie Name
* --------------------------------------------------------------------------
*
* Cookie name for Cross Site Request Forgery protection cookie.
*
* @var string
*/
public $cookieName = 'csrf_cookie_name';

/**
* --------------------------------------------------------------------------
* CSRF Expires
* --------------------------------------------------------------------------
*
* Expiration time for Cross Site Request Forgery protection cookie.
*
* Defaults to two hours (in seconds).
*
* @var integer
*/
public $expires = 7200;

/**
* --------------------------------------------------------------------------
* CSRF Regenerate
* --------------------------------------------------------------------------
*
* Regenerate CSRF Token on every request.
*
* @var boolean
*/
public $regenerate = true;

/**
* --------------------------------------------------------------------------
* CSRF Redirect
* --------------------------------------------------------------------------
*
* Redirect to previous page with error on failure.
*
* @var boolean
*/
public $redirect = true;

/**
* --------------------------------------------------------------------------
* 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
*
* @var string
*/
public $samesite = 'Lax';
}
12 changes: 3 additions & 9 deletions system/Common.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,7 @@ function config(string $name, bool $getShared = true)
*/
function csrf_token(): string
{
$config = config(App::class);

return $config->CSRFTokenName;
return Services::security()->getTokenName();
}
}

Expand All @@ -250,9 +248,7 @@ function csrf_token(): string
*/
function csrf_header(): string
{
$config = config(App::class);

return $config->CSRFHeaderName;
return Services::security()->getHeaderName();
}
}

Expand All @@ -267,9 +263,7 @@ function csrf_header(): string
*/
function csrf_hash(): string
{
$security = Services::security(null, true);

return $security->getCSRFHash();
return Services::security()->getHash();
}
}

Expand Down
4 changes: 1 addition & 3 deletions system/Config/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ public static function security(App $config = null, bool $getShared = true)
return static::getSharedInstance('security', $config);
}

$config = $config ?? config('App');
$config = $config ?? config('Security') ?? config('App');

return new Security($config);
}
Expand Down Expand Up @@ -859,6 +859,4 @@ public static function typography(bool $getShared = true)

return new Typography();
}

//--------------------------------------------------------------------
}
4 changes: 2 additions & 2 deletions system/Filters/CSRF.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ public function before(RequestInterface $request, $arguments = null)

try
{
$security->CSRFVerify($request);
$security->verify($request);
}
catch (SecurityException $e)
{
if (config('App')->CSRFRedirect && ! $request->isAJAX())
if ($security->shouldRedirect() && ! $request->isAJAX())
{
return redirect()->back()->with('error', $e->getMessage());
}
Expand Down
16 changes: 16 additions & 0 deletions system/Language/en/Security.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

/**
* This file is part of the CodeIgniter 4 framework.
*
* (c) CodeIgniter Foundation <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

// Security language settings
return [
'disallowedAction' => 'The action you requested is not allowed.',
'invalidSameSite' => 'The SameSite value must be None, Lax, Strict, or a blank string. Given: {0}',
];
8 changes: 4 additions & 4 deletions system/Security/Exceptions/SecurityException.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ class SecurityException extends FrameworkException
{
public static function forDisallowedAction()
{
return new static(lang('HTTP.disallowedAction'), 403);
return new static(lang('Security.disallowedAction'), 403);
}

public static function forInvalidSameSiteSetting(string $samesite)
public static function forInvalidSameSite(string $samesite)
{
return new static(lang('HTTP.invalidSameSiteSetting', [$samesite]));
return new static(lang('Security.invalidSameSite', [$samesite]));
}
}
Loading

0 comments on commit a2de1f6

Please sign in to comment.