From b1fa45ee3a20b730a9de1a4bb46b41d490f8d49a Mon Sep 17 00:00:00 2001 From: Samuel Georges Date: Sat, 2 Nov 2019 15:15:18 +1100 Subject: [PATCH] Combine common CSRF logic to a trait --- modules/backend/classes/Controller.php | 52 +---------- modules/cms/classes/Controller.php | 68 +------------- modules/system/traits/SecurityController.php | 98 ++++++++++++++++++++ 3 files changed, 100 insertions(+), 118 deletions(-) create mode 100644 modules/system/traits/SecurityController.php diff --git a/modules/backend/classes/Controller.php b/modules/backend/classes/Controller.php index 4d612fa0d..88ec3ef32 100644 --- a/modules/backend/classes/Controller.php +++ b/modules/backend/classes/Controller.php @@ -36,6 +36,7 @@ class Controller extends ControllerBase use \System\Traits\AssetMaker; use \System\Traits\ConfigMaker; use \System\Traits\EventEmitter; + use \System\Traits\SecurityController; use \Backend\Traits\ErrorMaker; use \Backend\Traits\WidgetMaker; use \October\Rain\Extension\ExtendableTrait; @@ -763,55 +764,4 @@ class Controller extends ControllerBase $hiddenHints = UserPreference::forUser()->get('backend::hints.hidden', []); return array_key_exists($name, $hiddenHints); } - - // - // Security - // - - /** - * Checks the request data / headers for a valid CSRF token. - * Returns false if a valid token is not found. Override this - * method to disable the check. - * @return bool - */ - protected function verifyCsrfToken() - { - if (!Config::get('cms.enableCsrfProtection')) { - return true; - } - - if (in_array(Request::method(), ['HEAD', 'GET', 'OPTIONS'])) { - return true; - } - - $token = Request::input('_token') ?: Request::header('X-CSRF-TOKEN'); - - if (!strlen($token) || !strlen(Session::token())) { - return false; - } - - return hash_equals( - Session::token(), - $token - ); - } - - /** - * Checks if the back-end should force a secure protocol (HTTPS) enabled by config. - * @return bool - */ - protected function verifyForceSecure() - { - if (Request::secure() || Request::ajax()) { - return true; - } - - // @todo if year >= 2018 change default from false to null - $forceSecure = Config::get('cms.backendForceSecure', false); - if ($forceSecure === null) { - $forceSecure = !Config::get('app.debug', false); - } - - return !$forceSecure; - } } diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 594e6f5a5..7910e0b89 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -6,14 +6,12 @@ use App; use View; use Lang; use Flash; -use Crypt; use Config; use Session; use Request; use Response; use Exception; use BackendAuth; -use Carbon\Carbon; use Twig\Environment as TwigEnvironment; use Twig\Cache\FilesystemCache as TwigCacheFilesystem; use Cms\Twig\Loader as TwigLoader; @@ -27,7 +25,6 @@ use System\Twig\Extension as SystemTwigExtension; use October\Rain\Exception\AjaxException; use October\Rain\Exception\ValidationException; use October\Rain\Parse\Bracket as TextParser; -use Symfony\Component\HttpFoundation\Cookie; use Illuminate\Http\RedirectResponse; use Symfony\Component\HttpFoundation\Response as BaseResponse; @@ -42,6 +39,7 @@ class Controller { use \System\Traits\AssetMaker; use \System\Traits\EventEmitter; + use \System\Traits\SecurityController; /** * @var \Cms\Classes\Theme A reference to the CMS theme processed by the controller. @@ -1605,68 +1603,4 @@ class Controller } } } - - // - // Security - // - - /** - * Adds anti-CSRF cookie. - * Adds a cookie with a token for CSRF checks to the response. - * - * @param BaseResponse $response The response object to add the cookie to - * @return BaseResponse - */ - protected function addXsrfCookie(BaseResponse $response) - { - $config = Config::get('session'); - - $response->headers->setCookie( - new Cookie( - 'XSRF-TOKEN', - Session::token(), - Carbon::now()->addMinutes((int) $config['lifetime'])->getTimestamp(), - $config['path'], - $config['domain'], - $config['secure'], - false, - false, - $config['same_site'] ?? null - ) - ); - - return $response; - } - - /** - * Checks the request data / headers for a valid CSRF token. - * Returns false if a valid token is not found. Override this - * method to disable the check. - * @return bool - */ - protected function verifyCsrfToken() - { - if (!Config::get('cms.enableCsrfProtection', true)) { - return true; - } - - if (in_array(Request::method(), ['HEAD', 'GET', 'OPTIONS'])) { - return true; - } - - $token = Request::input('_token') ?: Request::header('X-CSRF-TOKEN'); - - if (!$token && $header = Request::header('X-XSRF-TOKEN')) { - $token = Crypt::decrypt($header, false); - } - - if (!strlen($token) || !strlen(Session::token())) { - return false; - } - - return hash_equals( - Session::token(), - $token - ); - } } diff --git a/modules/system/traits/SecurityController.php b/modules/system/traits/SecurityController.php new file mode 100644 index 000000000..7c1d991a3 --- /dev/null +++ b/modules/system/traits/SecurityController.php @@ -0,0 +1,98 @@ +headers->setCookie( + new Cookie( + 'XSRF-TOKEN', + Session::token(), + Carbon::now()->addMinutes((int) $config['lifetime'])->getTimestamp(), + $config['path'], + $config['domain'], + $config['secure'], + false, + false, + $config['same_site'] ?? null + ) + ); + + return $response; + } + + /** + * Checks the request data / headers for a valid CSRF token. + * Returns false if a valid token is not found. Override this + * method to disable the check. + * @return bool + */ + protected function verifyCsrfToken() + { + if (!Config::get('cms.enableCsrfProtection', true)) { + return true; + } + + if (in_array(Request::method(), ['HEAD', 'GET', 'OPTIONS'])) { + return true; + } + + $token = Request::input('_token') ?: Request::header('X-CSRF-TOKEN'); + + if (!$token && $header = Request::header('X-XSRF-TOKEN')) { + $token = Crypt::decrypt($header, false); + } + + if (!strlen($token) || !strlen(Session::token())) { + return false; + } + + return hash_equals( + Session::token(), + $token + ); + } + + /** + * Checks if the back-end should force a secure protocol (HTTPS) enabled by config. + * @return bool + */ + protected function verifyForceSecure() + { + if (Request::secure() || Request::ajax()) { + return true; + } + + // @todo if year >= 2018 change default from false to null + $forceSecure = Config::get('cms.backendForceSecure', false); + if ($forceSecure === null) { + $forceSecure = !Config::get('app.debug', false); + } + + return !$forceSecure; + } +}