diff --git a/CHANGELOG-v3.2.md b/CHANGELOG-v3.2.md index 7ac359e60bd..d9bd3b14bd8 100644 --- a/CHANGELOG-v3.2.md +++ b/CHANGELOG-v3.2.md @@ -1,11 +1,20 @@ # Running Release Notes for Craft CMS 3.2 +::: warning +This update introduces a few changes in behavior to be aware of: + +- Custom login controllers must now explicitly set their `$allowAnonymous` values to include `self::ALLOW_ANONYMOUS_OFFLINE` if they wish to be available when the system is offline. +::: + ### Added - The `site` element query params now support passing multiple site handles, or `'*'`, to query elements across multiple sites at once. ([#2854](https://github.com/craftcms/cms/issues/2854)) - Added the `unique` element query param, which can be used to prevent duplicate elements when querying elements across multiple sites. +- Added `craft\web\Request::getIsLoginRequest()` and `craft\console\Request::getIsLoginRequest()`. ### Changed - Renamed `craft\helpers\ArrayHelper::filterByValue()` to `where()`. +- Anonymous/offline/Control Panel access validation now takes place from `craft\web\Controller::beforeAction()` rather than `craft\web\Application::handleRequest()`, giving controllers a chance to do things like set CORS headers before a `ForbiddenHttpException` or `ServiceUnavailableHttpException` is thrown. ([#4008](https://github.com/craftcms/cms/issues/4008)) +- Controllers can now set `$allowAnonymous` to a combination of bitwise integers `self::ALLOW_ANONYMOUS_LIVE` and `self::ALLOW_ANONYMOUS_OFFLINE`, or an array of action ID/bitwise integer pairs, to define whether their actions should be accessible anonymously even when the system is offline. ### Deprecated - Deprecated `craft\helpers\ArrayHelper::filterByValue()`. Use `where()` instead. diff --git a/src/console/Request.php b/src/console/Request.php index f5f7243e8c2..43d6e278639 100644 --- a/src/console/Request.php +++ b/src/console/Request.php @@ -81,6 +81,17 @@ public function getIsActionRequest(): bool return false; } + /** + * Returns whether this was a Login request. + * + * @return bool + * @since 3.2.0 + */ + public function getIsLoginRequest(): bool + { + return false; + } + /** * Returns whether the current request is solely an action request. (Narrator: It isn't.) */ diff --git a/src/controllers/AppController.php b/src/controllers/AppController.php index aaa6a0e86b2..29753ad87c3 100644 --- a/src/controllers/AppController.php +++ b/src/controllers/AppController.php @@ -42,7 +42,7 @@ class AppController extends Controller * @inheritdoc */ public $allowAnonymous = [ - 'migrate' + 'migrate' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, ]; // Public Methods diff --git a/src/controllers/BaseUpdaterController.php b/src/controllers/BaseUpdaterController.php index 086c40a29cc..3bc1880d180 100644 --- a/src/controllers/BaseUpdaterController.php +++ b/src/controllers/BaseUpdaterController.php @@ -50,7 +50,7 @@ abstract class BaseUpdaterController extends Controller /** * @inheritdoc */ - protected $allowAnonymous = true; + protected $allowAnonymous = self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE; /** * @var array The data associated with the current update diff --git a/src/controllers/TemplatesController.php b/src/controllers/TemplatesController.php index e0cb9df021f..60a38b8e858 100644 --- a/src/controllers/TemplatesController.php +++ b/src/controllers/TemplatesController.php @@ -47,11 +47,17 @@ class TemplatesController extends Controller */ public function beforeAction($action) { - $actionSegments = Craft::$app->getRequest()->getActionSegments(); + $request = Craft::$app->getRequest(); + $actionSegments = $request->getActionSegments(); if (isset($actionSegments[0]) && strtolower($actionSegments[0]) === 'templates') { throw new ForbiddenHttpException(); } + // Allow anonymous access to the Login template even if the site is offline + if ($request->getIsLoginRequest()) { + $this->allowAnonymous = self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE; + } + return parent::beforeAction($action); } diff --git a/src/controllers/UsersController.php b/src/controllers/UsersController.php index 29b52d036e6..762f6dd392a 100644 --- a/src/controllers/UsersController.php +++ b/src/controllers/UsersController.php @@ -90,14 +90,14 @@ class UsersController extends Controller * @inheritdoc */ protected $allowAnonymous = [ - 'login', - 'logout', - 'get-remaining-session-time', - 'send-password-reset-email', - 'send-activation-email', - 'save-user', - 'set-password', - 'verify-email' + 'get-remaining-session-time' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'login' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'logout' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'save-user' => self::ALLOW_ANONYMOUS_LIVE, + 'send-activation-email' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'send-password-reset-email' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'set-password' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, + 'verify-email' => self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE, ]; // Public Methods @@ -131,7 +131,6 @@ public function actionLogin() } if (!Craft::$app->getRequest()->getIsPost()) { - $this->_enforceOfflineLoginPage(); return null; } @@ -1640,7 +1639,6 @@ private function _handleLoginFailure(string $authError = null, User $user = null 'errorMessage' => $event->message, ]); - $this->_enforceOfflineLoginPage(); return null; } @@ -1682,27 +1680,6 @@ private function _handleSuccessfulLogin(bool $setNotice): Response return $this->redirectToPostedUrl($userSession->getIdentity(), $returnUrl); } - /** - * Ensures that either the site is online or the user is specifically requesting the login path. - * - * @throws ServiceUnavailableHttpException - */ - private function _enforceOfflineLoginPage() - { - if (!Craft::$app->getIsLive()) { - $request = Craft::$app->getRequest(); - if ($request->getIsCpRequest()) { - $loginPath = 'login'; - } else { - $loginPath = trim(Craft::$app->getConfig()->getGeneral()->getLoginPath(), '/'); - } - - if ($request->getPathInfo() !== $loginPath) { - throw new ServiceUnavailableHttpException(); - } - } - } - /** * Renders the Set Password template for a given user. * diff --git a/src/web/Application.php b/src/web/Application.php index 5de183b53c2..38f93eea6fe 100644 --- a/src/web/Application.php +++ b/src/web/Application.php @@ -151,11 +151,7 @@ public function setTimeZone($value) * * @param Request $request the request to be handled * @return Response the resulting response - * @throws HttpException - * @throws ServiceUnavailableHttpException - * @throws \craft\errors\DbConnectException - * @throws ForbiddenHttpException - * @throws \yii\web\NotFoundHttpException + * @throws \Throwable if reasons */ public function handleRequest($request): Response { @@ -248,33 +244,22 @@ public function handleRequest($request): Response return $this->_processConfigSyncLogic($request) ?: $this->getResponse(); } - // If the system is offline, make sure they have permission to be here - $this->_enforceSystemStatusPermissions($request); - + // If this is a plugin template request, make sure the user has access to the plugin // If this is a non-login, non-validate, non-setPassword CP request, make sure the user has access to the CP - if ($request->getIsCpRequest() && !($request->getIsActionRequest() && $this->_isSpecialCaseActionRequest($request))) { + if ( + $request->getIsCpRequest() && + !$request->getIsActionRequest() && + ($firstSeg = $request->getSegment(1)) !== null && + ($plugin = $this->getPlugins()->getPlugin($firstSeg)) !== null + ) { + /** @var Plugin $plugin */ $user = $this->getUser(); - - // Make sure the user has access to the CP if ($user->getIsGuest()) { return $user->loginRequired(); } - - if (!$user->checkPermission('accessCp')) { + if (!$user->checkPermission('accessPlugin-' . $plugin->id)) { throw new ForbiddenHttpException(); } - - // If they're accessing a plugin's section, make sure that they have permission to do so - $firstSeg = $request->getSegment(1); - - if ($firstSeg !== null) { - /** @var Plugin|null $plugin */ - $plugin = $this->getPlugins()->getPlugin($firstSeg); - - if ($plugin && !$user->checkPermission('accessPlugin-' . $plugin->id)) { - throw new ForbiddenHttpException(); - } - } } // If this is an action request, call the controller @@ -283,7 +268,12 @@ public function handleRequest($request): Response } // If we're still here, finally let Yii do it's thing. - return parent::handleRequest($request); + try { + return parent::handleRequest($request); + } catch (\Throwable $e) { + $this->_unregisterDebugModule(); + throw $e; + } } /** @@ -547,7 +537,7 @@ private function _processInstallRequest(Request $request) * * @param Request $request * @return Response|null - * @throws NotFoundHttpException if the requested action route is invalid + * @throws \Throwable if reasons */ private function _processActionRequest(Request $request) { @@ -557,55 +547,19 @@ private function _processActionRequest(Request $request) try { Craft::debug("Route requested: '$route'", __METHOD__); $this->requestedRoute = $route; - return $this->runAction($route, $_GET); - } catch (InvalidRouteException $e) { - throw new NotFoundHttpException(Craft::t('yii', 'Page not found.'), $e->getCode(), $e); + } catch (\Throwable $e) { + $this->_unregisterDebugModule(); + if ($e instanceof InvalidRouteException) { + throw new NotFoundHttpException(Craft::t('yii', 'Page not found.'), $e->getCode(), $e); + } + throw $e; } } return null; } - /** - * Returns whether this is a special case request (something dealing with user sessions or updating) - * where system status / CP permissions shouldn't be taken into effect. - * - * @param Request $request - * @return bool - */ - private function _isSpecialCaseActionRequest(Request $request): bool - { - $actionSegs = $request->getActionSegments(); - - if (empty($actionSegs)) { - return false; - } - - return ( - $actionSegs === ['app', 'migrate'] || - $actionSegs === ['users', 'login'] || - $actionSegs === ['users', 'forgot-password'] || - $actionSegs === ['users', 'send-password-reset-email'] || - $actionSegs === ['users', 'get-remaining-session-time'] || - ( - $request->getIsSingleActionRequest() && - ( - $actionSegs === ['users', 'logout'] || - $actionSegs === ['users', 'set-password'] || - $actionSegs === ['users', 'verify-email'] - ) - ) || - ( - $request->getIsCpRequest() && - ( - $actionSegs[0] === 'update' || - $actionSegs[0] === 'manualupdate' - ) - ) - ); - } - /** * If there is not cached app path or the existing cached app path does not match the current one, let’s run the * requirement checker again. This should catch the case where an install is deployed to another server that doesn’t @@ -752,50 +706,4 @@ private function _handleIncompatibleConfig(Request $request): Response // TemplatesController->actionRenderError() take care of it. throw new ServiceUnavailableHttpException(); } - - /** - * Checks if the system is off, and if it is, enforces the "Access the site/CP when the system is off" permissions. - * - * @param Request $request - * @throws ServiceUnavailableHttpException - */ - private function _enforceSystemStatusPermissions(Request $request) - { - if (!$this->_checkSystemStatusPermissions($request)) { - $error = null; - - if (!$this->getUser()->getIsGuest()) { - if ($request->getIsCpRequest()) { - $error = Craft::t('app', 'Your account doesn’t have permission to access the Control Panel when the system is offline.'); - } else { - $error = Craft::t('app', 'Your account doesn’t have permission to access the site when the system is offline.'); - } - } else { - // If this is a CP request, redirect to the Login page - if ($this->getRequest()->getIsCpRequest()) { - $this->getUser()->loginRequired(); - $this->end(); - } - } - - $this->_unregisterDebugModule(); - throw new ServiceUnavailableHttpException($error); - } - } - - /** - * Returns whether the user has permission to be accessing the site/CP while it's offline, if it is. - * - * @param Request $request - * @return bool - */ - private function _checkSystemStatusPermissions(Request $request): bool - { - if ($this->getIsLive() || $this->_isSpecialCaseActionRequest($request)) { - return true; - } - - $permission = $request->getIsCpRequest() ? 'accessCpWhenSystemIsOff' : 'accessSiteWhenSystemIsOff'; - return $this->getUser()->checkPermission($permission); - } } diff --git a/src/web/Controller.php b/src/web/Controller.php index 46b3bd16160..aff4f15bb40 100644 --- a/src/web/Controller.php +++ b/src/web/Controller.php @@ -14,6 +14,7 @@ use GuzzleHttp\Exception\ClientException; use yii\base\Action; use yii\base\InvalidArgumentException; +use yii\base\InvalidConfigException; use yii\web\BadRequestHttpException; use yii\web\ForbiddenHttpException; use yii\web\HttpException; @@ -31,23 +32,71 @@ */ abstract class Controller extends \yii\web\Controller { + // Constants + // ========================================================================= + + const ALLOW_ANONYMOUS_NEVER = 0; + const ALLOW_ANONYMOUS_LIVE = 1; + const ALLOW_ANONYMOUS_OFFLINE = 2; + // Properties // ========================================================================= /** - * @var bool|string[] Whether this controller’s actions can be accessed anonymously - * If set to false, you are required to be logged in to execute any of the given controller's actions. - * If set to true, anonymous access is allowed for all of the given controller's actions. - * If the value is an array of action IDs, then you must be logged in for any actions except for the ones in - * the array list. - * If you have a controller that where the majority of actions allow anonymous access, but you only want require - * login on a few, you can set this to true and call [[requireLogin()]] in the individual methods. + * @var int|bool|int[]|string[] Whether this controller’s actions can be accessed anonymously. + * + * This can be set to any of the following: + * + * - `false` or `self::ALLOW_ANONYMOUS_NEVER` (default) – indicates that all controller actions should never be + * accessed anonymously + * - `true` or `self::ALLOW_ANONYMOUS_LIVE` – indicates that all controller actions can be accessed anonymously when + * the system is live + * - `self::ALLOW_ANONYMOUS_OFFLINE` – indicates that all controller actions can be accessed anonymously when the + * system is offline + * - `self::ALLOW_ANONYMOUS_LIVE | self::ALLOW_ANONYMOUS_OFFLINE` – indicates that all controller actions can be + * accessed anonymously when the system is live or offline + * - An array of action IDs (e.g. `['save-guest-entry', 'edit-guest-entry']`) – indicates that the listed action IDs + * can be accessed anonymously when the system is live + * - An array of action ID/bitwise pairs (e.g. `['save-guest-entry' => self::ALLOW_ANONYMOUS_OFFLINE]` – indicates + * that the listed action IDs can be accessed anonymously per the bitwise int assigned to it. */ - protected $allowAnonymous = false; + protected $allowAnonymous = self::ALLOW_ANONYMOUS_NEVER; // Public Methods // ========================================================================= + /** + * @inheritdoc + * @throws InvalidConfigException if [[$allowAnonymous]] is set to an invalid value + */ + public function init() + { + // Normalize $allowAnonymous + if (is_bool($this->allowAnonymous)) { + $this->allowAnonymous = (int)$this->allowAnonymous; + } else if (is_array($this->allowAnonymous)) { + $normalized = []; + foreach ($this->allowAnonymous as $k => $v) { + if ( + (is_int($k) && !is_string($v)) || + (is_string($k) && !is_int($v)) + ) { + throw new InvalidArgumentException("Invalid \$allowAnonymous value for key \"{$k}\""); + } + if (is_int($k)) { + $normalized[$v] = self::ALLOW_ANONYMOUS_LIVE; + } else { + $normalized[$k] = $v; + } + } + $this->allowAnonymous = $normalized; + } else if (!is_int($this->allowAnonymous)) { + throw new InvalidConfigException('Invalid $allowAnonymous value'); + } + + parent::init(); + } + /** * This method is invoked right before an action is executed. * @@ -77,11 +126,16 @@ abstract class Controller extends \yii\web\Controller * * @param Action $action the action to be executed. * @return bool whether the action should continue to run. + * @throws BadRequestHttpException if the request is missing a valid CSRF token + * @throws ForbiddenHttpException if the user is not logged in or locks the necessary permissions + * @throws ServiceUnavailableHttpException if the system is offline and the user isn't allowed to access it */ public function beforeAction($action) { + $request = Craft::$app->getRequest(); + // Don't enable CSRF validation for Live Preview requests - if (Craft::$app->getRequest()->getIsLivePreview()) { + if ($request->getIsLivePreview()) { $this->enableCsrfValidation = false; } @@ -90,11 +144,34 @@ public function beforeAction($action) } // Enforce $allowAnonymous - if ( - (is_array($this->allowAnonymous) && (!preg_grep("/{$action->id}/i", $this->allowAnonymous))) || - $this->allowAnonymous === false - ) { - $this->requireLogin(); + $isLive = Craft::$app->getIsLive(); + $test = $isLive ? self::ALLOW_ANONYMOUS_LIVE : self::ALLOW_ANONYMOUS_OFFLINE; + + if (is_int($this->allowAnonymous)) { + $allowAnonymous = $this->allowAnonymous; + } else { + $allowAnonymous = $this->allowAnonymous[$action->id] ?? self::ALLOW_ANONYMOUS_NEVER; + } + + if (!($test & $allowAnonymous)) { + // If this is a CP request, make sure they have access to the CP + if ($request->getIsCpRequest()) { + $this->requireLogin(); + $this->requirePermission('accessCp'); + } else if (Craft::$app->getUser()->getIsGuest()) { + throw new ServiceUnavailableHttpException(); + } + + // If the system is offline, make sure they have permission to access the CP/site + if (!$isLive) { + $permission = $request->getIsCpRequest() ? 'accessCpWhenSystemIsOff' : 'accessSiteWhenSystemIsOff'; + if (!Craft::$app->getUser()->checkPermission($permission)) { + $error = $request->getIsCpRequest() + ? Craft::t('app', 'Your account doesn’t have permission to access the Control Panel when the system is offline.') + : Craft::t('app', 'Your account doesn’t have permission to access the site when the system is offline.'); + throw new ServiceUnavailableHttpException($error); + } + } } return true; diff --git a/src/web/Request.php b/src/web/Request.php index 4b949ec0ab5..0361e9f8aae 100644 --- a/src/web/Request.php +++ b/src/web/Request.php @@ -99,6 +99,11 @@ class Request extends \yii\web\Request */ private $_isSingleActionRequest = false; + /** + * @var bool + */ + private $_isLoginRequest = false; + /** * @var bool */ @@ -435,6 +440,18 @@ public function getIsActionRequest(): bool return $this->_isActionRequest; } + /** + * Returns whether this was a Login request. + * + * @return bool + * @since 3.2.0 + */ + public function getIsLoginRequest(): bool + { + $this->_checkRequestType(); + return $this->_isLoginRequest; + } + /** * Returns whether the current request is solely an action request. */ @@ -1209,6 +1226,7 @@ private function _checkRequestType() switch ($this->_path) { case $loginPath: $this->_actionSegments = ['users', 'login']; + $this->_isLoginRequest = true; break; case $logoutPath: $this->_actionSegments = ['users', 'logout'];