From cc397da1bec21d27fbf9ff392958298014630b9c Mon Sep 17 00:00:00 2001
From: Bjoern Schiessle <schiessle@owncloud.com>
Date: Mon, 15 Feb 2016 16:24:21 +0100
Subject: [PATCH] Remove background job if the server accepted to ask for the
 shared secret

If we don't remove it the server will later ask the remote server to ask for
the shared secret which will result in a error log message on the remote server
and in some circumstances maybe even to a failure
---
 apps/federation/api/ocsauthapi.php           | 25 ++++++++++++++++----
 apps/federation/tests/api/ocsauthapitest.php |  3 +++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/apps/federation/api/ocsauthapi.php b/apps/federation/api/ocsauthapi.php
index 058a596637..1c4e73cc8d 100644
--- a/apps/federation/api/ocsauthapi.php
+++ b/apps/federation/api/ocsauthapi.php
@@ -33,7 +33,6 @@ use OCP\BackgroundJob\IJobList;
 use OCP\ILogger;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
-use OCP\Security\StringUtils;
 
 /**
  * Class OCSAuthAPI
@@ -99,7 +98,7 @@ class OCSAuthAPI {
 		$token = $this->request->getParam('token');
 
 		if ($this->trustedServers->isTrustedServer($url) === false) {
-			$this->logger->log(\OCP\Util::ERROR, 'remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']);
+			$this->logger->error('remote server not trusted (' . $url . ') while requesting shared secret', ['app' => 'federation']);
 			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
 		}
 
@@ -107,10 +106,22 @@ class OCSAuthAPI {
 		// token wins
 		$localToken = $this->dbHandler->getToken($url);
 		if (strcmp($localToken, $token) > 0) {
-			$this->logger->log(\OCP\Util::ERROR, 'remote server (' . $url . ') presented lower token', ['app' => 'federation']);
+			$this->logger->info(
+				'remote server (' . $url . ') presented lower token. We will initiate the exchange of the shared secret.',
+				['app' => 'federation']
+			);
 			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
 		}
 
+		// we ask for the shared secret so we no longer have to ask the other server
+		// to request the shared secret
+		$this->jobList->remove('OCA\Federation\BackgroundJob\RequestSharedSecret',
+			[
+				'url' => $url,
+				'token' => $localToken
+			]
+		);
+
 		$this->jobList->add(
 			'OCA\Federation\BackgroundJob\GetSharedSecret',
 			[
@@ -134,12 +145,16 @@ class OCSAuthAPI {
 		$token = $this->request->getParam('token');
 
 		if ($this->trustedServers->isTrustedServer($url) === false) {
-			$this->logger->log(\OCP\Util::ERROR, 'remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']);
+			$this->logger->error('remote server not trusted (' . $url . ') while getting shared secret', ['app' => 'federation']);
 			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
 		}
 
 		if ($this->isValidToken($url, $token) === false) {
-			$this->logger->log(\OCP\Util::ERROR, 'remote server (' . $url . ') didn\'t send a valid token (got ' . $token . ') while getting shared secret', ['app' => 'federation']);
+			$expectedToken = $this->dbHandler->getToken($url);
+			$this->logger->error(
+				'remote server (' . $url . ') didn\'t send a valid token (got "' . $token . '" but expected "'. $expectedToken . '") while getting shared secret',
+				['app' => 'federation']
+			);
 			return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
 		}
 
diff --git a/apps/federation/tests/api/ocsauthapitest.php b/apps/federation/tests/api/ocsauthapitest.php
index 9c751fff89..d3e61c0641 100644
--- a/apps/federation/tests/api/ocsauthapitest.php
+++ b/apps/federation/tests/api/ocsauthapitest.php
@@ -105,8 +105,11 @@ class OCSAuthAPITest extends TestCase {
 		if ($expected === Http::STATUS_OK) {
 			$this->jobList->expects($this->once())->method('add')
 				->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token]);
+			$this->jobList->expects($this->once())->method('remove')
+				->with('OCA\Federation\BackgroundJob\RequestSharedSecret', ['url' => $url, 'token' => $localToken]);
 		} else {
 			$this->jobList->expects($this->never())->method('add');
+			$this->jobList->expects($this->never())->method('remove');
 		}
 
 		$result = $this->ocsAuthApi->requestSharedSecret();