From faf6a65e07ed5be648f265dfde71d44e0fe3e767 Mon Sep 17 00:00:00 2001
From: Arthur Schiwon <blizzz@arthur-schiwon.de>
Date: Wed, 10 Apr 2024 21:45:33 +0200
Subject: [PATCH] fix(federation): give some time to prepare both servers

- when this background job runs, while the current server was not being
  added as trusted_server in the other instance, yet, the secret sharing
  would not be attempted again, without visual feedback.
- the change allows 5 attempts, which gives more than 20minutes to
  complete. More do not really help as the endpoint is brute force
  protected.

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
---
 .../lib/BackgroundJob/RequestSharedSecret.php          | 10 ++++++++--
 .../tests/BackgroundJob/RequestSharedSecretTest.php    |  8 +++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
index 98eb81000f256..f8c25168b56ce 100644
--- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
+++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php
@@ -160,7 +160,7 @@ protected function run($argument) {
 		// if we received a unexpected response we try again later
 		if (
 			$status !== Http::STATUS_OK
-			&& $status !== Http::STATUS_FORBIDDEN
+			&& ($status !== Http::STATUS_FORBIDDEN || $this->getAttempt($argument) < 5)
 		) {
 			$this->retainJob = true;
 		}
@@ -173,14 +173,20 @@ protected function reAddJob(array $argument): void {
 		$url = $argument['url'];
 		$created = isset($argument['created']) ? (int)$argument['created'] : $this->time->getTime();
 		$token = $argument['token'];
+		$attempt = $this->getAttempt($argument) + 1;
 
 		$this->jobList->add(
 			RequestSharedSecret::class,
 			[
 				'url' => $url,
 				'token' => $token,
-				'created' => $created
+				'created' => $created,
+				'attempt' => $attempt
 			]
 		);
 	}
+
+	protected function getAttempt(array $argument): int {
+		return $argument['attempt'] ?? 0;
+	}
 }
diff --git a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
index 5aca6005f94f9..059348aa8ab0a 100644
--- a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
+++ b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php
@@ -142,6 +142,7 @@ public function testStart($isTrustedServer, $retainBackgroundJob) {
 						'url' => 'url',
 						'token' => 'token',
 						'created' => 42,
+						'attempt' => 1,
 					]
 				);
 		} else {
@@ -164,12 +165,12 @@ public function dataTestStart() {
 	 *
 	 * @param int $statusCode
 	 */
-	public function testRun($statusCode) {
+	public function testRun(int $statusCode, int $attempt = 0): void {
 		$target = 'targetURL';
 		$source = 'sourceURL';
 		$token = 'token';
 
-		$argument = ['url' => $target, 'token' => $token];
+		$argument = ['url' => $target, 'token' => $token, 'attempt' => $attempt];
 
 		$this->timeFactory->method('getTime')->willReturn(42);
 
@@ -196,7 +197,7 @@ public function testRun($statusCode) {
 		$this->invokePrivate($this->requestSharedSecret, 'run', [$argument]);
 		if (
 			$statusCode !== Http::STATUS_OK
-			&& $statusCode !== Http::STATUS_FORBIDDEN
+			&& ($statusCode !== Http::STATUS_FORBIDDEN || $attempt < 5)
 		) {
 			$this->assertTrue($this->invokePrivate($this->requestSharedSecret, 'retainJob'));
 		} else {
@@ -207,6 +208,7 @@ public function testRun($statusCode) {
 	public function dataTestRun() {
 		return [
 			[Http::STATUS_OK],
+			[Http::STATUS_FORBIDDEN, 5],
 			[Http::STATUS_FORBIDDEN],
 			[Http::STATUS_CONFLICT],
 		];