Anonymous/offline/CP access validation refactoring

Resolves #4008
craftcms · Mar 19, 2019 · 93de503 · 93de503
1 parent a16f297
commit 93de503
Show file tree

Hide file tree

Showing 9 changed files with 169 additions and 163 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
-    }
 }