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