From 60cf5ea297a17db861e73cd65d7b7862bd6bcc24 Mon Sep 17 00:00:00 2001 From: Inverle Date: Thu, 4 Dec 2025 08:46:11 +0100 Subject: Improve anonymous authentication logic (#8165) * Improve anonymous authentication logic * forgot to git add * Fix incorrect token check Because an empty parameter could be just passed if token for the user wasn't set: `&token=` --- app/Controllers/feedController.php | 9 +-------- app/Controllers/indexController.php | 12 ++---------- app/Models/Auth.php | 14 ++------------ app/layout/layout.phtml | 12 ++++++++---- lib/Minz/Request.php | 16 ++++++++++++++++ 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/app/Controllers/feedController.php b/app/Controllers/feedController.php index b6ecbeec2..1829417c1 100644 --- a/app/Controllers/feedController.php +++ b/app/Controllers/feedController.php @@ -13,12 +13,6 @@ class FreshRSS_feed_Controller extends FreshRSS_ActionController { #[\Override] public function firstAction(): void { if (!FreshRSS_Auth::hasAccess()) { - // Token is useful in the case that anonymous refresh is forbidden - // and CRON task cannot be used with php command so the user can - // set a CRON task to refresh his feeds by using token inside url - $token = FreshRSS_Context::userConf()->token; - $token_param = Minz_Request::paramString('token'); - $token_is_ok = ($token != '' && $token == $token_param); $action = Minz_Request::actionName(); $allow_anonymous_refresh = FreshRSS_Context::systemConf()->allow_anonymous_refresh; @@ -28,8 +22,7 @@ class FreshRSS_feed_Controller extends FreshRSS_ActionController { return; } - if ($action !== 'actualize' || - !($allow_anonymous_refresh || $token_is_ok)) { + if ($action !== 'actualize' || !$allow_anonymous_refresh) { Minz_Error::error(403); } } diff --git a/app/Controllers/indexController.php b/app/Controllers/indexController.php index bfa1eb521..fa46c3f3a 100644 --- a/app/Controllers/indexController.php +++ b/app/Controllers/indexController.php @@ -200,14 +200,9 @@ class FreshRSS_index_Controller extends FreshRSS_ActionController { */ public function rssAction(): void { $allow_anonymous = FreshRSS_Context::systemConf()->allow_anonymous; - $token = FreshRSS_Context::userConf()->token; - $token_param = Minz_Request::paramString('token'); - $token_is_ok = ($token != '' && $token === $token_param); // Check if user has access. - if (!FreshRSS_Auth::hasAccess() && - !$allow_anonymous && - !$token_is_ok) { + if (!FreshRSS_Auth::hasAccess() && !$allow_anonymous) { Minz_Error::error(403); } @@ -241,12 +236,9 @@ class FreshRSS_index_Controller extends FreshRSS_ActionController { */ public function opmlAction(): void { $allow_anonymous = FreshRSS_Context::systemConf()->allow_anonymous; - $token = FreshRSS_Context::userConf()->token; - $token_param = Minz_Request::paramString('token'); - $token_is_ok = ($token != '' && $token === $token_param); // Check if user has access. - if (!FreshRSS_Auth::hasAccess() && !$allow_anonymous && !$token_is_ok) { + if (!FreshRSS_Auth::hasAccess() && !$allow_anonymous) { Minz_Error::error(403); } diff --git a/app/Models/Auth.php b/app/Models/Auth.php index 6bf4a2b3f..ee806d78b 100644 --- a/app/Models/Auth.php +++ b/app/Models/Auth.php @@ -170,18 +170,8 @@ class FreshRSS_Auth { 'REMOTE_USER' => false, ]); - $username = ''; - $token_param = Minz_Request::paramString('token'); - if ($token_param != '') { - $username = Minz_Request::paramString('user'); - if ($username != '') { - $conf = FreshRSS_UserConfiguration::getForUser($username); - if ($conf == null) { - $username = ''; - } - } - } - if ($username == '') { + $username = Minz_Request::paramString('user'); + if (!Minz_Request::tokenIsOk()) { $username = FreshRSS_Context::systemConf()->default_user; } Minz_User::change($username); diff --git a/app/layout/layout.phtml b/app/layout/layout.phtml index fc6675a40..aa89cac96 100644 --- a/app/layout/layout.phtml +++ b/app/layout/layout.phtml @@ -53,8 +53,10 @@ if ($this->rss_title != '') { $url_rss = $url_base; $url_rss['a'] = 'rss'; - $url_rss['params']['user'] = Minz_User::name() ?? ''; - $url_rss['params']['token'] = FreshRSS_Context::userConf()->token; + if (FreshRSS_Auth::hasAccess()) { + $url_rss['params']['user'] = Minz_User::name() ?? ''; + $url_rss['params']['token'] = FreshRSS_Context::userConf()->token; + } unset($url_rss['params']['rid']); if (FreshRSS_Context::userConf()->since_hours_posts_per_rss) { $url_rss['params']['hours'] = FreshRSS_Context::userConf()->since_hours_posts_per_rss; @@ -64,8 +66,10 @@ token; + if (FreshRSS_Auth::hasAccess()) { + $opml_rss['params']['user'] = Minz_User::name() ?? ''; + $opml_rss['params']['token'] = FreshRSS_Context::userConf()->token; + } unset($opml_rss['params']['rid']); ?> diff --git a/lib/Minz/Request.php b/lib/Minz/Request.php index 3355058f1..0e8dc28d0 100644 --- a/lib/Minz/Request.php +++ b/lib/Minz/Request.php @@ -560,6 +560,22 @@ class Minz_Request { return 'POST' === ($_SERVER['REQUEST_METHOD'] ?? ''); } + public static function tokenIsOk(): bool { + $token_param = self::paramString('token'); + if ($token_param == '') { + return false; + } + $username = self::paramString('user'); + if ($username == '') { + return false; + } + $conf = FreshRSS_UserConfiguration::getForUser($username); + if ($conf === null || !hash_equals($conf->token, $token_param)) { + return false; + } + return true; + } + /** * @return list */ -- cgit v1.2.3