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

@@ -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);
 			}
 		}

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);
 		}
 

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);

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) ?>" />

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>
 	 */