From f73d8e6d498a211192976a525016b4a961707e3f Mon Sep 17 00:00:00 2001 From: Samuel Georges Date: Sat, 2 Nov 2019 16:16:32 +1100 Subject: [PATCH 1/2] Removes double middleware layer For some reason it was decided to allow October controllers to support Laravel middleware, this has been reverted because it is a convoluted solution that doesn't respect the original architecture. There are other ways to handle middleware requirements The original use case appeared to be to simply allow backend controllers to inject headers. This is something easily solvable whilst keeping the simple and original workflow --- modules/backend/classes/BackendController.php | 109 ++---------------- modules/backend/classes/Controller.php | 40 +------ modules/backend/controllers/Auth.php | 38 ++++-- 3 files changed, 39 insertions(+), 148 deletions(-) diff --git a/modules/backend/classes/BackendController.php b/modules/backend/classes/BackendController.php index 24a54465a..d72cc491c 100644 --- a/modules/backend/classes/BackendController.php +++ b/modules/backend/classes/BackendController.php @@ -62,44 +62,6 @@ class BackendController extends ControllerBase */ public function __construct() { - $this->middleware(function ($request, $next) { - // Process the request before retrieving controller middleware, to allow for the session and auth data - // to be made available to the controller's constructor. - $response = $next($request); - - // Find requested controller to determine if any middleware has been attached - $pathParts = explode('/', str_replace(Request::root() . '/', '', Request::url())); - if (count($pathParts)) { - // Drop off preceding backend URL part if needed - if (!empty(Config::get('cms.backendUri', 'backend'))) { - array_shift($pathParts); - } - $path = implode('/', $pathParts); - - $requestedController = $this->getRequestedController($path); - if ( - !is_null($requestedController) - && is_array($requestedController) - && count($requestedController['controller']->getMiddleware()) - ) { - $action = $requestedController['action']; - - // Collect applicable middleware and insert middleware into pipeline - $controllerMiddleware = collect($requestedController['controller']->getMiddleware()) - ->reject(function ($data) use ($action) { - return static::methodExcludedByOptions($action, $data['options']); - }) - ->pluck('middleware'); - - foreach ($controllerMiddleware as $middleware) { - $middleware->call($requestedController['controller'], $request, $response); - } - } - } - - return $response; - }); - $this->extendableConstruct(); } @@ -125,7 +87,8 @@ class BackendController extends ControllerBase ) { $this->cmsHandling = true; return App::make('Cms\Classes\Controller')->run($url); - } else { + } + else { return Response::make(View::make('backend::404'), 404); } } @@ -158,34 +121,6 @@ class BackendController extends ControllerBase : $this->passToCmsController($url); } - $controllerRequest = $this->getRequestedController($url); - if (!is_null($controllerRequest)) { - return $controllerRequest['controller']->run( - $controllerRequest['action'], - $controllerRequest['params'] - ); - } - - /* - * Fall back on Cms controller - */ - return $this->passToCmsController($url); - } - - /** - * Determines the controller and action to load in the backend via a provided URL. - * - * If a suitable controller is found, this will return an array with the controller class name as a string, the - * action to call as a string and an array of parameters. If a suitable controller and action cannot be found, - * this method will return null. - * - * @param string $url A URL to determine the requested controller and action for - * @return array|null A suitable controller, action and parameters in an array if found, otherwise null. - */ - protected function getRequestedController($url) - { - $params = RouterHelper::segmentizeUrl($url); - /* * Look for a Module controller */ @@ -199,11 +134,7 @@ class BackendController extends ControllerBase $action, base_path().'/modules' )) { - return [ - 'controller' => $controllerObj, - 'action' => $action, - 'params' => $controllerParams - ]; + return $controllerObj->run($action, $controllerParams); } /* @@ -226,15 +157,14 @@ class BackendController extends ControllerBase $action, plugins_path() )) { - return [ - 'controller' => $controllerObj, - 'action' => $action, - 'params' => $controllerParams - ]; + return $controllerObj->run($action, $controllerParams); } } - return null; + /* + * Fall back on Cms controller + */ + return $this->passToCmsController($url); } /** @@ -247,10 +177,6 @@ class BackendController extends ControllerBase */ protected function findController($controller, $action, $inPath) { - if (isset($this->requestedController)) { - return $this->requestedController; - } - /* * Workaround: Composer does not support case insensitivity. */ @@ -263,16 +189,16 @@ class BackendController extends ControllerBase } if (!class_exists($controller)) { - return $this->requestedController = null; + return false; } $controllerObj = App::make($controller); if ($controllerObj->actionExists($action)) { - return $this->requestedController = $controllerObj; + return $controllerObj; } - return $this->requestedController = null; + return false; } /** @@ -288,17 +214,4 @@ class BackendController extends ControllerBase return $actionName; } - - /** - * Determine if the given options exclude a particular method. - * - * @param string $method - * @param array $options - * @return bool - */ - protected static function methodExcludedByOptions($method, array $options) - { - return (isset($options['only']) && !in_array($method, (array) $options['only'])) || - (!empty($options['except']) && in_array($method, (array) $options['except'])); - } } diff --git a/modules/backend/classes/Controller.php b/modules/backend/classes/Controller.php index 4d612fa0d..7021d5e31 100644 --- a/modules/backend/classes/Controller.php +++ b/modules/backend/classes/Controller.php @@ -19,9 +19,9 @@ use October\Rain\Exception\AjaxException; use October\Rain\Exception\SystemException; use October\Rain\Exception\ValidationException; use October\Rain\Exception\ApplicationException; +use October\Rain\Extension\Extendable; use Illuminate\Database\Eloquent\MassAssignmentException; use Illuminate\Http\RedirectResponse; -use Illuminate\Routing\Controller as ControllerBase; /** * The Backend base controller class, used by Backend controllers. @@ -30,7 +30,7 @@ use Illuminate\Routing\Controller as ControllerBase; * @package october\backend * @author Alexey Bobkov, Samuel Georges */ -class Controller extends ControllerBase +class Controller extends Extendable { use \System\Traits\ViewMaker; use \System\Traits\AssetMaker; @@ -38,12 +38,6 @@ class Controller extends ControllerBase use \System\Traits\EventEmitter; use \Backend\Traits\ErrorMaker; use \Backend\Traits\WidgetMaker; - use \October\Rain\Extension\ExtendableTrait; - - /** - * @var array Behaviors implemented by this controller. - */ - public $implement; /** * @var object Reference the logged in admin user. @@ -165,36 +159,6 @@ class Controller extends ControllerBase $manager = new MediaManager($this, 'ocmediamanager'); $manager->bindToController(); } - - $this->extendableConstruct(); - } - - /** - * Extend this object properties upon construction. - */ - public static function extend(Closure $callback) - { - self::extendableExtendCallback($callback); - } - - public function __get($name) - { - return $this->extendableGet($name); - } - - public function __set($name, $value) - { - $this->extendableSet($name, $value); - } - - public function __call($name, $params) - { - return $this->extendableCall($name, $params); - } - - public static function __callStatic($name, $params) - { - return self::extendableCallStatic($name, $params); } /** diff --git a/modules/backend/controllers/Auth.php b/modules/backend/controllers/Auth.php index 1f37a3d06..b02313c87 100644 --- a/modules/backend/controllers/Auth.php +++ b/modules/backend/controllers/Auth.php @@ -3,6 +3,7 @@ use Mail; use Flash; use Backend; +use Request; use Validator; use BackendAuth; use Backend\Models\AccessLog; @@ -34,18 +35,18 @@ class Auth extends Controller { parent::__construct(); - $this->middleware(function ($request, $response) { - // Clear Cache and any previous data to fix Invalid security token issue, see github: #3707 - $response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate'); - })->only('signin'); + // $this->middleware(function ($request, $response) { + // // Clear Cache and any previous data to fix Invalid security token issue, see github: #3707 + // $response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate'); + // })->only('signin'); - // Only run on HTTPS connections - if (isset($_SERVER["HTTPS"]) && $_SERVER["HTTPS"] === "on") { - $this->middleware(function ($request, $response) { - // Add HTTP Header 'Clear Site Data' to remove all Sensitive Data when signout, see github issue: #3707 - $response->headers->set('Clear-Site-Data', 'cache, cookies, storage, executionContexts'); - })->only('signout'); - } + // // Only run on HTTPS connections + // if (isset($_SERVER["HTTPS"]) && $_SERVER["HTTPS"] === "on") { + // $this->middleware(function ($request, $response) { + // // Add HTTP Header 'Clear Site Data' to remove all Sensitive Data when signout, see github issue: #3707 + // $response->headers->set('Clear-Site-Data', 'cache, cookies, storage, executionContexts'); + // })->only('signout'); + // } $this->layout = 'auth'; } @@ -129,7 +130,14 @@ class Auth extends Controller BackendAuth::logout(); } - return Backend::redirect('backend'); + $redirect = Backend::redirect('backend'); + + // Add HTTP Header 'Clear Site Data' to purge all sensitive data upon signout + if (Request::secure()) { + $redirect->header('Clear-Site-Data', 'cache, cookies, storage, executionContexts'); + } + + return $redirect; } /** @@ -146,6 +154,9 @@ class Auth extends Controller } } + /** + * Submits the restore form. + */ public function restore_onSubmit() { $rules = [ @@ -202,6 +213,9 @@ class Auth extends Controller $this->vars['id'] = $userId; } + /** + * Submits the reset form. + */ public function reset_onSubmit() { if (!post('id') || !post('code')) { From 92bd8360b9e05af0191afe8f4159df2e22e330e6 Mon Sep 17 00:00:00 2001 From: Samuel Georges Date: Sat, 2 Nov 2019 16:30:33 +1100 Subject: [PATCH 2/2] Fixes issue where behaviors are not booting --- modules/backend/classes/BackendController.php | 9 +-------- modules/backend/classes/Controller.php | 2 ++ modules/backend/controllers/Auth.php | 8 -------- 3 files changed, 3 insertions(+), 16 deletions(-) diff --git a/modules/backend/classes/BackendController.php b/modules/backend/classes/BackendController.php index d72cc491c..5043b069e 100644 --- a/modules/backend/classes/BackendController.php +++ b/modules/backend/classes/BackendController.php @@ -8,10 +8,10 @@ use Event; use Config; use Request; use Response; -use Closure; use Illuminate\Routing\Controller as ControllerBase; use October\Rain\Router\Helper as RouterHelper; use System\Classes\PluginManager; +use Closure; /** * This is the master controller for all back-end pages. @@ -50,13 +50,6 @@ class BackendController extends ControllerBase */ protected $cmsHandling = false; - /** - * Stores the requested controller so that the constructor is only run once - * - * @var Backend\Classes\Controller - */ - protected $requestedController; - /** * Instantiate a new BackendController instance. */ diff --git a/modules/backend/classes/Controller.php b/modules/backend/classes/Controller.php index 7021d5e31..ead8a6c8f 100644 --- a/modules/backend/classes/Controller.php +++ b/modules/backend/classes/Controller.php @@ -152,6 +152,8 @@ class Controller extends Extendable */ $this->user = BackendAuth::getUser(); + parent::__construct(); + /* * Media Manager widget is available on all back-end pages */ diff --git a/modules/backend/controllers/Auth.php b/modules/backend/controllers/Auth.php index b02313c87..1c8c4403f 100644 --- a/modules/backend/controllers/Auth.php +++ b/modules/backend/controllers/Auth.php @@ -40,14 +40,6 @@ class Auth extends Controller // $response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate'); // })->only('signin'); - // // Only run on HTTPS connections - // if (isset($_SERVER["HTTPS"]) && $_SERVER["HTTPS"] === "on") { - // $this->middleware(function ($request, $response) { - // // Add HTTP Header 'Clear Site Data' to remove all Sensitive Data when signout, see github issue: #3707 - // $response->headers->set('Clear-Site-Data', 'cache, cookies, storage, executionContexts'); - // })->only('signout'); - // } - $this->layout = 'auth'; }