From 63f65a3f252fe7f13da4967fb0d39a8bee550f76 Mon Sep 17 00:00:00 2001 From: Samuel Georges Date: Sat, 2 Nov 2019 19:14:45 +1100 Subject: [PATCH] Add XSRF to backend, simplify CMS controller run() method runInternal has been removed because we do not want to blanket our response logic over every single response, only the happy path. This is because it is impossible to remove. So it is better to take the inverted approach, where if you want the CMS' headers in your custom response, add them yourself. This becomes easy via the new makeResponse() method --- modules/backend/classes/Controller.php | 7 +++++ modules/cms/classes/Controller.php | 28 +++++-------------- modules/system/traits/ResponseMaker.php | 26 +++++++++++++----- modules/system/traits/SecurityController.php | 29 ++++++++------------ 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/modules/backend/classes/Controller.php b/modules/backend/classes/Controller.php index 05d4eb563..fe929add1 100644 --- a/modules/backend/classes/Controller.php +++ b/modules/backend/classes/Controller.php @@ -174,6 +174,13 @@ class Controller extends Extendable return Response::make(Lang::get('system::lang.page.invalid_token.label'), 403); } + if ( + Config::get('cms.enableCsrfProtection', true) && + Config::get('cms.enableXsrfCookies', true) + ) { + $this->setResponseCookie($this->makeXsrfCookie()); + } + /* * Check forced HTTPS protocol. * @see \System\Traits\SecurityController diff --git a/modules/cms/classes/Controller.php b/modules/cms/classes/Controller.php index 53293d70e..0e2a38da2 100644 --- a/modules/cms/classes/Controller.php +++ b/modules/cms/classes/Controller.php @@ -138,27 +138,6 @@ class Controller * @return BaseResponse Returns the response to the provided URL */ public function run($url = '/') - { - $response = $this->runInternal($url); - - if ( - Config::get('cms.enableCsrfProtection', true) && - Config::get('cms.enableXsrfCookies', true) && - $response instanceof BaseResponse - ) { - $this->addXsrfCookie($response); - } - - return $response; - } - - /** - * Process the request internally - * - * @param string $url Specifies the requested page URL. - * @return BaseResponse Returns the response to the provided URL - */ - protected function runInternal($url = '/') { if ($url === null) { $url = Request::path(); @@ -176,6 +155,13 @@ class Controller return Response::make(Lang::get('system::lang.page.invalid_token.label'), 403); } + if ( + Config::get('cms.enableCsrfProtection', true) && + Config::get('cms.enableXsrfCookies', true) + ) { + $this->setResponseCookie($this->makeXsrfCookie()); + } + /* * Hidden page */ diff --git a/modules/system/traits/ResponseMaker.php b/modules/system/traits/ResponseMaker.php index a4c9ff1de..f9690c910 100644 --- a/modules/system/traits/ResponseMaker.php +++ b/modules/system/traits/ResponseMaker.php @@ -1,7 +1,7 @@ statusCode = (int) $code; + return $this; } @@ -57,6 +58,7 @@ trait ResponseMaker public function setResponse($response) { $this->responseOverride = $response; + return $this; } @@ -71,7 +73,7 @@ trait ResponseMaker public function setResponseHeader($key, $values, $replace = true) { if ($this->responseHeaderBag === null) { - $this->responseHeaderBag = new HeaderBag; + $this->responseHeaderBag = new ResponseHeaderBag; } $this->responseHeaderBag->set($key, $values, $replace); @@ -88,7 +90,7 @@ trait ResponseMaker public function setResponseCookie($cookie) { if ($this->responseHeaderBag === null) { - $this->responseHeaderBag = new HeaderBag; + $this->responseHeaderBag = new ResponseHeaderBag; } if (is_string($cookie) && function_exists('cookie')) { @@ -100,6 +102,15 @@ trait ResponseMaker return $this; } + /** + * Get the header response bag + * @return Symfony\Component\HttpFoundation\ResponseHeaderBag|null + */ + public function getResponseHeaders() + { + return $this->responseHeaderBag; + } + /** * Prepares a response that considers overrides and custom responses. * @param mixed $contents @@ -112,11 +123,12 @@ trait ResponseMaker } if (is_string($contents)) { - $contents = Response::make($contents, $this->statusCode); + $contents = Response::make($contents, $this->getStatusCode()); } - if ($contents instanceof BaseResponse && $this->responseHeaderBag !== null) { - $contents = $contents->withHeaders($this->responseHeaderBag); + $responseHeaders = $this->getResponseHeaders(); + if ($responseHeaders && $contents instanceof BaseResponse) { + $contents = $contents->withHeaders($responseHeaders); } return $contents; diff --git a/modules/system/traits/SecurityController.php b/modules/system/traits/SecurityController.php index 7c1d991a3..d96a7ca16 100644 --- a/modules/system/traits/SecurityController.php +++ b/modules/system/traits/SecurityController.php @@ -21,28 +21,23 @@ trait SecurityController * 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 + * @return \Symfony\Component\HttpFoundation\Cookie */ - protected function addXsrfCookie(BaseResponse $response) + protected function makeXsrfCookie() { $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 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; } /**