fix: Add RequestSharedSecret job if local server should start the exchange

When the local server is asked to request the shared secret from a
remote server the local server refuses to do it if the local server has
a higher token and is therefore the one expected to start the shared
secret exchange. However, if the local server no longer had a
RequestSharedSecret job the local server never started the exchange.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>

apps/federation/lib/Controller/OCSAuthAPIController.php

@@ -9,6 +9,7 @@ namespace OCA\Federation\Controller;
 
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
+use OCA\Federation\BackgroundJob\RequestSharedSecret;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Attribute\BruteForceProtection;
 use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
@@ -109,6 +110,9 @@ class OCSAuthAPIController extends OCSController {
 			$this->logger->info(
 				'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.'
 			);
+
+			$this->addRequestSharedSecretJobIfNeeded($url, $localToken);
+
 			throw new OCSForbiddenException();
 		}
 
@@ -124,6 +128,50 @@ class OCSAuthAPIController extends OCSController {
 		return new DataResponse();
 	}
 
+	/**
+	 * Adds a RequestSharedSecret job for the given URL if it does not exist.
+	 *
+	 * @param string $url the URL to add the job for.
+	 * @param string $token the local token for the trusted server URL.
+	 */
+	private function addRequestSharedSecretJobIfNeeded(string $url, string $token) {
+		if ($this->hasRequestSharedSecretJobFor($url)) {
+			return;
+		}
+
+		$this->logger->info(
+			'No RequestSharedSecret job for ' . $url . ', a new one will be added to initiate the exchange of the shared secret.'
+		);
+
+		$this->jobList->add(
+			RequestSharedSecret::class,
+			[
+				'url' => $url,
+				'token' => $token,
+				'created' => $this->timeFactory->getTime()
+			]
+		);
+	}
+
+	/**
+	 * Returns whether there is a RequestSharedSecret job for the given URL or
+	 * not.
+	 *
+	 * Other arguments of the job are not taken into account.
+	 *
+	 * @param string $url the URL to check if there is a job for it.
+	 * @return bool True if there is a job, false otherwise.
+	 */
+	private function hasRequestSharedSecretJobFor(string $url) {
+		foreach ($this->jobList->getJobsIterator(RequestSharedSecret::class, null, 0) as $job) {
+			if (is_array($job->getArgument()) && strcmp($job->getArgument()['url'], $url) === 0) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * Create shared secret and return it
 	 *

apps/federation/tests/Controller/OCSAuthAPIControllerTest.php

@@ -10,11 +10,13 @@ namespace OCA\Federation\Tests\Controller;
 
 use OC\BackgroundJob\JobList;
 use OCA\Federation\BackgroundJob\GetSharedSecret;
+use OCA\Federation\BackgroundJob\RequestSharedSecret;
 use OCA\Federation\Controller\OCSAuthAPIController;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\AppFramework\OCS\OCSForbiddenException;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\BackgroundJob\IJob;
 use OCP\IRequest;
 use OCP\Security\Bruteforce\IThrottler;
 use OCP\Security\ISecureRandom;
@@ -65,7 +67,7 @@ class OCSAuthAPIControllerTest extends TestCase {
 	}
 
 	#[\PHPUnit\Framework\Attributes\DataProvider('dataTestRequestSharedSecret')]
-	public function testRequestSharedSecret(string $token, string $localToken, bool $isTrustedServer, bool $ok): void {
+	public function testRequestSharedSecret(string $token, string $localToken, bool $isTrustedServer, ?bool $hasRequestSharedSecretJob, bool $ok): void {
 		$url = 'url';
 
 		$this->trustedServers
@@ -74,9 +76,37 @@ class OCSAuthAPIControllerTest extends TestCase {
 		$this->dbHandler->expects($this->any())
 			->method('getToken')->with($url)->willReturn($localToken);
 
+		if (strcmp($token, $localToken) < 0 && $isTrustedServer) {
+			$this->jobList->expects($this->once())->method('getJobsIterator')
+				->with(RequestSharedSecret::class, null, 0)
+				->willReturnCallback(function() use ($hasRequestSharedSecretJob) {
+					$jobWithoutArguments = $this->createMock(IJob::class);
+					$jobWithoutArguments->method('getArgument')->willReturn(null);
+					$jobForAnotherServer = $this->createMock(IJob::class);
+					$jobForAnotherServer->method('getArgument')->willReturn(['url' => 'other url']);
+					$data = [
+						$jobWithoutArguments,
+						$jobForAnotherServer,
+					];
+
+					if ($hasRequestSharedSecretJob) {
+						$jobForSameServer = $this->createMock(IJob::class);
+						$jobForSameServer->method('getArgument')->willReturn(['url' => 'url']);
+						array_push($data, $jobForSameServer);
+					}
+
+					foreach ($data as $element) {
+						yield $element;
+					}
+				});
+		}
+
 		if ($ok) {
 			$this->jobList->expects($this->once())->method('add')
 				->with(GetSharedSecret::class, ['url' => $url, 'token' => $token, 'created' => $this->currentTime]);
+		} elseif (strcmp($token, $localToken) < 0 && $isTrustedServer && !$hasRequestSharedSecretJob) {
+			$this->jobList->expects($this->once())->method('add')
+				->with(RequestSharedSecret::class, ['url' => $url, 'token' => $localToken, 'created' => $this->currentTime]);
 		} else {
 			$this->jobList->expects($this->never())->method('add');
 			$this->jobList->expects($this->never())->method('remove');
@@ -98,9 +128,10 @@ class OCSAuthAPIControllerTest extends TestCase {
 
 	public static function dataTestRequestSharedSecret(): array {
 		return [
-			['token2', 'token1', true, true],
-			['token1', 'token2', false, false],
-			['token1', 'token2', true, false],
+			['token2', 'token1', true, null, true],
+			['token1', 'token2', true, true, false],
+			['token1', 'token2', true, false, false],
+			['token1', 'token2', false, null, false],
 		];
 	}