Move ban checking from WCF boot to middleware
authorTim Düsterhus <duesterhus@woltlab.com>
Mon, 8 Aug 2022 09:25:13 +0000 (11:25 +0200)
committerTim Düsterhus <duesterhus@woltlab.com>
Mon, 8 Aug 2022 09:29:03 +0000 (11:29 +0200)
The previous location in WCF made sense back when `initBlacklist()` also
checked the IP address, User-Agent, or hostname blocklist to prevent processing
the request as much as possible. As all these checks are gone now, the only
thing that remains is the inexpensive ban check.

Move it into a middleware, it does not really belong into WCF which should just
be responsible for booting the framework independently of the request in
question.

wcfsetup/install/files/lib/http/middleware/CheckUserBan.class.php [new file with mode: 0644]
wcfsetup/install/files/lib/system/WCF.class.php
wcfsetup/install/files/lib/system/WCFACP.class.php
wcfsetup/install/files/lib/system/request/RequestHandler.class.php

diff --git a/wcfsetup/install/files/lib/http/middleware/CheckUserBan.class.php b/wcfsetup/install/files/lib/http/middleware/CheckUserBan.class.php
new file mode 100644 (file)
index 0000000..3b3ed41
--- /dev/null
@@ -0,0 +1,67 @@
+<?php
+
+namespace wcf\http\middleware;
+
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\ServerRequestInterface;
+use Psr\Http\Server\MiddlewareInterface;
+use Psr\Http\Server\RequestHandlerInterface;
+use wcf\data\user\User;
+use wcf\system\exception\AJAXException;
+use wcf\system\exception\NamedUserException;
+use wcf\system\WCF;
+
+/**
+ * Checks whether the user is banned and deletes their sessions.
+ *
+ * @author  Tim Duesterhus
+ * @copyright   2001-2022 WoltLab GmbH
+ * @license GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
+ * @package WoltLabSuite\Core\Http\Middleware
+ * @since   6.0
+ */
+final class CheckUserBan implements MiddlewareInterface
+{
+    /**
+     * @inheritDoc
+     */
+    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
+    {
+        $user = WCF::getUser();
+
+        if ($this->isBanned($user)) {
+            if ($this->isAjaxRequest($request)) {
+                throw new AJAXException(
+                    WCF::getLanguage()->getDynamicVariable('wcf.user.error.isBanned'),
+                    AJAXException::INSUFFICIENT_PERMISSIONS
+                );
+            } else {
+                // Delete sessions only for non-AJAX requests to ensure
+                // that the user was able to see the message properly
+                WCF::getSession()->deleteUserSessionsExcept($user);
+
+                throw new NamedUserException(WCF::getLanguage()->getDynamicVariable('wcf.user.error.isBanned'));
+            }
+        }
+
+        return $handler->handle($request);
+    }
+
+    private function isBanned(User $user): bool
+    {
+        if (!$user->userID) {
+            return false;
+        }
+
+        if ($user->hasOwnerAccess()) {
+            return false;
+        }
+
+        return !!$user->banned;
+    }
+
+    private function isAjaxRequest(ServerRequestInterface $request): bool
+    {
+        return $request->getHeaderLine('x-requested-with') === 'XMLHttpRequest';
+    }
+}
index 6b58ccd6f770965f1a12d9d495d30332a630e5f2..6cad036d54273bbdbfb7e250158634f91b25ebe3 100644 (file)
@@ -18,10 +18,8 @@ use wcf\system\cache\builder\PackageUpdateCacheBuilder;
 use wcf\system\cronjob\CronjobScheduler;
 use wcf\system\database\MySQLDatabase;
 use wcf\system\event\EventHandler;
-use wcf\system\exception\AJAXException;
 use wcf\system\exception\ErrorException;
 use wcf\system\exception\IPrintableException;
-use wcf\system\exception\NamedUserException;
 use wcf\system\exception\ParentClassException;
 use wcf\system\exception\SystemException;
 use wcf\system\language\LanguageFactory;
@@ -177,7 +175,6 @@ class WCF
         $this->initCronjobs();
         $this->initCoreObjects();
         $this->initApplications();
-        $this->initBlacklist();
 
         EventHandler::getInstance()->fireAction($this, 'initialized');
     }
@@ -561,28 +558,6 @@ class WCF
         $styleHandler->changeStyle($styleID);
     }
 
-    /**
-     * Executes the blacklist.
-     */
-    protected function initBlacklist()
-    {
-        $isAjax = isset($_SERVER['HTTP_X_REQUESTED_WITH']) && ($_SERVER['HTTP_X_REQUESTED_WITH'] == 'XMLHttpRequest');
-
-        // handle banned users
-        if (self::getUser()->userID && self::getUser()->banned && !self::getUser()->hasOwnerAccess()) {
-            if ($isAjax) {
-                throw new AJAXException(
-                    self::getLanguage()->getDynamicVariable('wcf.user.error.isBanned'),
-                    AJAXException::INSUFFICIENT_PERMISSIONS
-                );
-            } else {
-                self::getSession()->delete();
-
-                throw new NamedUserException(self::getLanguage()->getDynamicVariable('wcf.user.error.isBanned'));
-            }
-        }
-    }
-
     /**
      * Initializes applications.
      */
index 13548a5ad881207081268d1bb44f5dd72b3b8683..f5fc42760ef29cc361a677c2373f87cbca068a44 100644 (file)
@@ -69,7 +69,6 @@ class WCFACP extends WCF
             $this->initApplications();
         }
 
-        $this->initBlacklist();
         $this->initAuth();
 
         EventHandler::getInstance()->fireAction($this, 'initialized');
index 4ed1d04ce18e0817a9159aab9a17d304c87847b8..383219a65f27bd95775cf807459038d310c0e7ee 100644 (file)
@@ -14,6 +14,7 @@ use wcf\http\middleware\CheckForEnterpriseNonOwnerAccess;
 use wcf\http\middleware\CheckForExpiredAppEvaluation;
 use wcf\http\middleware\CheckForOfflineMode;
 use wcf\http\middleware\CheckSystemEnvironment;
+use wcf\http\middleware\CheckUserBan;
 use wcf\http\middleware\EnforceCacheControlPrivate;
 use wcf\http\middleware\EnforceFrameOptions;
 use wcf\http\Pipeline;
@@ -95,6 +96,7 @@ final class RequestHandler extends SingletonFactory
                     new EnforceCacheControlPrivate(),
                     new EnforceFrameOptions(),
                     new CheckSystemEnvironment(),
+                    new CheckUserBan(),
                     new CheckForEnterpriseNonOwnerAccess(),
                     new CheckForExpiredAppEvaluation(),
                     new CheckForOfflineMode(),