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=`

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 @@
 <?php } if (FreshRSS_Context::isAll() || FreshRSS_Context::isAllAndCategories() || FreshRSS_Context::isAllAndArchived() || FreshRSS_Context::isCategory() || FreshRSS_Context::isFeed()) {
 		$opml_rss = $url_base;
 		$opml_rss['a'] = 'opml';
-		$opml_rss['params']['user'] = Minz_User::name() ?? '';
-		$opml_rss['params']['token'] = FreshRSS_Context::userConf()->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']);
 ?>
 		<link rel="outline" type="text/x-opml" title="OPML" href="<?= Minz_Url::display($opml_rss) ?>" />
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<string>
 	 */