From 687a2d0d34aed0022120865fb9b78ec12f2bc080 Mon Sep 17 00:00:00 2001 From: Rhilip Date: Wed, 14 Aug 2019 23:56:57 +0800 Subject: [PATCH] refactor(Auth): Fix Certification process 1. Fix Certification process 2. Move getBanIpList to IpBanMiddleware --- apps/components/Auth.php | 2 +- apps/components/Site.php | 8 --- apps/config/http_base.php | 5 -- apps/middleware/AuthMiddleware.php | 108 ++++++++++++---------------- apps/middleware/IpBanMiddleware.php | 20 +++++- 5 files changed, 66 insertions(+), 77 deletions(-) diff --git a/apps/components/Auth.php b/apps/components/Auth.php index 3b60b4d..bf8f333 100644 --- a/apps/components/Auth.php +++ b/apps/components/Auth.php @@ -36,7 +36,7 @@ public function onRequestAfter() /** * @param string $grant - * @param bool $flush + * @param string|bool $flush * @return models\User|bool return False means this user is anonymous */ public function getCurUser($grant = 'cookies', $flush = false) diff --git a/apps/components/Site.php b/apps/components/Site.php index 8943a56..e3b5525 100644 --- a/apps/components/Site.php +++ b/apps/components/Site.php @@ -45,14 +45,6 @@ protected function getCacheNameSpace(): string return 'Site:hash:runtime_value'; } - public function getBanIpsList(): array - { - // FIXME `ip_ban_list` will still left in redis cache, it means if you change the ip_ban_list, it may not available in time. - return $this->getCacheValue('ip_ban_list', function () { - return app()->pdo->createCommand('SELECT `ip` FROM `ban_ips`')->queryColumn(); - }); - } - public function getTorrent($tid) { if (array_key_exists($tid, $this->torrents)) { diff --git a/apps/config/http_base.php b/apps/config/http_base.php index 2934315..31af313 100644 --- a/apps/config/http_base.php +++ b/apps/config/http_base.php @@ -39,11 +39,6 @@ 'GET captcha' => ['captcha', 'index'], 'GET maintenance' => ['maintenance', 'index'], - // Auth By Passkey Route - 'GET rss' => ['rss', 'index', 'middleware' => [ - apps\middleware\AuthMiddleware::class - ]], - // API version 1 'api/v1/{controller}/{action}' => ['api/v1/{controller}', '{action}', 'middleware' => [ apps\middleware\ApiMiddleware::class, diff --git a/apps/middleware/AuthMiddleware.php b/apps/middleware/AuthMiddleware.php index 0506ca2..342e386 100644 --- a/apps/middleware/AuthMiddleware.php +++ b/apps/middleware/AuthMiddleware.php @@ -14,8 +14,8 @@ class AuthMiddleware { const authByPasskeyAction = [ - [controllers\RssController::class, 'actionIndex'], - [controllers\TorrentController::class, 'actionDownload'] + [controllers\RssController::class, 'actionIndex'], // `/rss?passkey=` + [controllers\TorrentController::class, 'actionDownload'] // `/torrent/download?passkey=` ]; /** @noinspection PhpUnused */ @@ -27,7 +27,7 @@ public function handle($callable, \Closure $next) // Try auth by cookies first $curuser = app()->auth->getCurUser('cookies', true); - // if fails and in special route `/rss?passkey=` or `/torrent/download?passkey=` , then try auth by passkey + // Try auth by passkey in special route and action if first cookies-check fails if ($curuser === false) { foreach (self::authByPasskeyAction as $value) { list($_controller, $_action) = $value; @@ -38,72 +38,56 @@ public function handle($callable, \Closure $next) } } - if (config('base.prevent_anonymous') && $curuser === false) return app()->response->setStatusCode(403); - if (config('base.maintenance') && !$curuser->isPrivilege('bypass_maintenance')) return app()->response->redirect('/maintenance'); - return $this->{'authBy' . ucfirst(app()->auth->getGrant())}($callable, $next); - } - - /** @noinspection PhpUnusedPrivateMethodInspection */ - private function authByPasskey($callable, \Closure $next) { - if (false === $curuser = app()->auth->getCurUser('passkey')) { - return 'invalid Passkey'; - } - return $next(); - } + // Check if Site in Maintenance status, and only let `bypass_maintenance` user access + if (config('base.maintenance') && ($curuser === false || !$curuser->isPrivilege('bypass_maintenance'))) + return app()->response->redirect('/maintenance'); - /** @noinspection PhpUnusedPrivateMethodInspection */ - private function authByCookies($callable, \Closure $next) { - list($controller, $action) = $callable; - $controllerName = get_class($controller); + // Deal with Anonymous Visitor + if ($curuser === false) { + // Check if Site in Abnormal status + if (config('base.prevent_anonymous')) return app()->response->setStatusCode(403); - $curuser = app()->auth->getCurUser(); + if (app()->auth->getGrant() == 'passkey') { + return 'invalid Passkey'; + } else { // app()->auth->getGrant() == 'cookies' + // If visitor want to auth himself + if ($controllerName === controllers\AuthController::class && $action !== 'actionLogout') return $next(); - $now_ip = app()->request->getClientIp(); - if ($controllerName === controllers\AuthController::class) { - if ($curuser !== false && in_array($action, ['actionLogin', 'actionRegister', 'actionConfirm'])) { - /** Don't allow Logged in user visit the auth/{login, register, confirm} */ - return app()->response->redirect('/index'); - } elseif ($action !== 'actionLogout') { - if ($action == 'actionLogin') { // TODO add register confirm fail ip count check - $test_count = app()->redis->hGet('Site:fail_login_ip_count', $now_ip) ?: 0; - if ($test_count > config('security.max_login_attempts')) { - return app()->response->setStatusCode(403); - } - } - return $next(); + // Prevent Other Route + app()->cookie->delete(Constant::cookie_name); // Delete exist cookies + app()->session->set('login_return_to', app()->request->fullUrl()); // Store the url which visitor want to hit + return app()->response->redirect('/auth/login'); } } - if (false === $curuser) { - app()->cookie->delete(Constant::cookie_name); // Delete exist cookies - app()->session->set('login_return_to', app()->request->fullUrl()); // Store the url which visitor want to hit - return app()->response->redirect('/auth/login'); - } else { - /** - * TODO move to Auth Component - * Check User Permission to this route - * - * When user visit - /admin -> Controller : \apps\controllers\AdminController Action: actionIndex - * it will check the dynamic config key `authority.route_admin_index` and compare with curuser class , - * if user don't have this permission to visit this route the http code 403 will throw out. - * if this config key is not exist , the default class 1 will be used to compare. - * - * Example of `Route - Controller - Config Key` Map: - * /admin -> AdminController::actionIndex -> route.admin_index - * /admin/service -> AdminController::actionService -> route.admin_service - */ - $route = strtolower(str_replace( - ['apps\\controllers\\', 'Controller', 'action'], '', - $controllerName . '_' . $action - ) - ); - $required_class = config('route.' . $route) ?: 1; - if ($curuser->getClass() < $required_class) { - return app()->response->setStatusCode(403); // FIXME redirect to /error may better - } + // Don't allow Logged in user visit the auth/{login, register, confirm} + if ($controllerName === controllers\AuthController::class && + in_array($action, ['actionLogin', 'actionRegister', 'actionConfirm'])) { + return app()->response->redirect('/index'); + } + + /** + * Check User Permission to this route + * + * When user visit - /admin -> Controller : \apps\controllers\AdminController Action: actionIndex + * it will check the dynamic config key `authority.route_admin_index` and compare with curuser class , + * if user don't have this permission to visit this route the http code 403 will throw out. + * if this config key is not exist , the default class 1 will be used to compare. + * + * Example of `Route - Controller - Config Key` Map: + * /admin -> AdminController::actionIndex -> route.admin_index + * /admin/service -> AdminController::actionService -> route.admin_service + */ + $route = strtolower(str_replace( + ['apps\\controllers\\', 'Controller', 'action'], '', + $controllerName . '_' . $action + ) + ); + $required_class = config('route.' . $route) ?: 1; + if ($curuser->getClass() < $required_class) { + return app()->response->setStatusCode(403); // FIXME redirect to /error may better } - // 执行下一个中间件 - return $next(); + return $next(); // 执行下一个中间件 } } diff --git a/apps/middleware/IpBanMiddleware.php b/apps/middleware/IpBanMiddleware.php index e2f1c65..bb65490 100644 --- a/apps/middleware/IpBanMiddleware.php +++ b/apps/middleware/IpBanMiddleware.php @@ -12,10 +12,11 @@ class IpBanMiddleware { + /** @noinspection PhpUnused */ public function handle($callable, \Closure $next) { $ip = app()->request->getClientIp(); - $ip_ban_list = app()->site->getBanIpsList(); + $ip_ban_list = $this->getBanIpsList(); if (count($ip_ban_list) > 0 && IpUtils::checkIp($ip, $ip_ban_list)) { return app()->response->setStatusCode(403); @@ -23,4 +24,21 @@ public function handle($callable, \Closure $next) return $next(); } + + private function getBanIpsList(): array + { + $timenow = time(); + $ban_ips_check = config('runtime.ban_ips_list_check'); + if ($ban_ips_check === false // Init to avoid Redis Cache not exist + || $ban_ips_check > $timenow + 86400 // Keep Redis Cache For 1 days + ) { + $ban_ips = app()->pdo->createCommand('SELECT `ip` FROM `ban_ips`')->queryColumn() ?: []; + app()->redis->sAdd('Site:ban_ips_list', ...$ban_ips); + app()->config->set('runtime.ban_ips_list_check', $timenow, 'int'); + } else { + $ban_ips = app()->redis->sMembers('Site:ban_ips_list'); + } + + return $ban_ips; + } }