From 51154ba3f8f1d09b54560d5d1933f9053ef409cb Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 8 Aug 2022 11:25:13 +0200 Subject: [PATCH] Move ban checking from WCF boot to middleware 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. --- .../http/middleware/CheckUserBan.class.php | 67 +++++++++++++++++++ .../install/files/lib/system/WCF.class.php | 25 ------- .../install/files/lib/system/WCFACP.class.php | 1 - .../system/request/RequestHandler.class.php | 2 + 4 files changed, 69 insertions(+), 26 deletions(-) create mode 100644 wcfsetup/install/files/lib/http/middleware/CheckUserBan.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 index 0000000000..3b3ed410bc --- /dev/null +++ b/wcfsetup/install/files/lib/http/middleware/CheckUserBan.class.php @@ -0,0 +1,67 @@ + + * @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'; + } +} diff --git a/wcfsetup/install/files/lib/system/WCF.class.php b/wcfsetup/install/files/lib/system/WCF.class.php index 6b58ccd6f7..6cad036d54 100644 --- a/wcfsetup/install/files/lib/system/WCF.class.php +++ b/wcfsetup/install/files/lib/system/WCF.class.php @@ -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. */ diff --git a/wcfsetup/install/files/lib/system/WCFACP.class.php b/wcfsetup/install/files/lib/system/WCFACP.class.php index 13548a5ad8..f5fc42760e 100644 --- a/wcfsetup/install/files/lib/system/WCFACP.class.php +++ b/wcfsetup/install/files/lib/system/WCFACP.class.php @@ -69,7 +69,6 @@ class WCFACP extends WCF $this->initApplications(); } - $this->initBlacklist(); $this->initAuth(); EventHandler::getInstance()->fireAction($this, 'initialized'); diff --git a/wcfsetup/install/files/lib/system/request/RequestHandler.class.php b/wcfsetup/install/files/lib/system/request/RequestHandler.class.php index 4ed1d04ce1..383219a65f 100644 --- a/wcfsetup/install/files/lib/system/request/RequestHandler.class.php +++ b/wcfsetup/install/files/lib/system/request/RequestHandler.class.php @@ -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(), -- 2.20.1