Merge pull request #22403 from owncloud/improved-error-handling
Remove background job if the server accepted to ask for the shared secret
This commit is contained in:
commit
e5641247a3
|
@ -33,7 +33,6 @@ use OCP\BackgroundJob\IJobList;
|
||||||
use OCP\ILogger;
|
use OCP\ILogger;
|
||||||
use OCP\IRequest;
|
use OCP\IRequest;
|
||||||
use OCP\Security\ISecureRandom;
|
use OCP\Security\ISecureRandom;
|
||||||
use OCP\Security\StringUtils;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Class OCSAuthAPI
|
* Class OCSAuthAPI
|
||||||
|
@ -99,7 +98,7 @@ class OCSAuthAPI {
|
||||||
$token = $this->request->getParam('token');
|
$token = $this->request->getParam('token');
|
||||||
|
|
||||||
if ($this->trustedServers->isTrustedServer($url) === false) {
|
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);
|
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -107,10 +106,22 @@ class OCSAuthAPI {
|
||||||
// token wins
|
// token wins
|
||||||
$localToken = $this->dbHandler->getToken($url);
|
$localToken = $this->dbHandler->getToken($url);
|
||||||
if (strcmp($localToken, $token) > 0) {
|
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);
|
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(
|
$this->jobList->add(
|
||||||
'OCA\Federation\BackgroundJob\GetSharedSecret',
|
'OCA\Federation\BackgroundJob\GetSharedSecret',
|
||||||
[
|
[
|
||||||
|
@ -134,12 +145,16 @@ class OCSAuthAPI {
|
||||||
$token = $this->request->getParam('token');
|
$token = $this->request->getParam('token');
|
||||||
|
|
||||||
if ($this->trustedServers->isTrustedServer($url) === false) {
|
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);
|
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||||
}
|
}
|
||||||
|
|
||||||
if ($this->isValidToken($url, $token) === false) {
|
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);
|
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -150,10 +150,14 @@ class GetSharedSecret extends QueuedJob{
|
||||||
|
|
||||||
} catch (ClientException $e) {
|
} catch (ClientException $e) {
|
||||||
$status = $e->getCode();
|
$status = $e->getCode();
|
||||||
$this->logger->logException($e);
|
if ($status === Http::STATUS_FORBIDDEN) {
|
||||||
|
$this->logger->info($target . ' refused to exchange a shared secret with you.', ['app' => 'federation']);
|
||||||
|
} else {
|
||||||
|
$this->logger->logException($e, ['app' => 'federation']);
|
||||||
|
}
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
|
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
|
||||||
$this->logger->logException($e);
|
$this->logger->logException($e, ['app' => 'federation']);
|
||||||
}
|
}
|
||||||
|
|
||||||
// if we received a unexpected response we try again later
|
// if we received a unexpected response we try again later
|
||||||
|
|
|
@ -148,10 +148,14 @@ class RequestSharedSecret extends QueuedJob {
|
||||||
|
|
||||||
} catch (ClientException $e) {
|
} catch (ClientException $e) {
|
||||||
$status = $e->getCode();
|
$status = $e->getCode();
|
||||||
$this->logger->logException($e);
|
if ($status === Http::STATUS_FORBIDDEN) {
|
||||||
|
$this->logger->info($target . ' refused to ask for a shared secret.', ['app' => 'federation']);
|
||||||
|
} else {
|
||||||
|
$this->logger->logException($e, ['app' => 'federation']);
|
||||||
|
}
|
||||||
} catch (\Exception $e) {
|
} catch (\Exception $e) {
|
||||||
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
|
$status = HTTP::STATUS_INTERNAL_SERVER_ERROR;
|
||||||
$this->logger->logException($e);
|
$this->logger->logException($e, ['app' => 'federation']);
|
||||||
}
|
}
|
||||||
|
|
||||||
// if we received a unexpected response we try again later
|
// if we received a unexpected response we try again later
|
||||||
|
|
|
@ -156,6 +156,7 @@ class DbHandler {
|
||||||
*
|
*
|
||||||
* @param string $url
|
* @param string $url
|
||||||
* @return string
|
* @return string
|
||||||
|
* @throws \Exception
|
||||||
*/
|
*/
|
||||||
public function getToken($url) {
|
public function getToken($url) {
|
||||||
$hash = $this->hash($url);
|
$hash = $this->hash($url);
|
||||||
|
@ -165,6 +166,11 @@ class DbHandler {
|
||||||
->setParameter('url_hash', $hash);
|
->setParameter('url_hash', $hash);
|
||||||
|
|
||||||
$result = $query->execute()->fetch();
|
$result = $query->execute()->fetch();
|
||||||
|
|
||||||
|
if (!isset($result['token'])) {
|
||||||
|
throw new \Exception('No token found for: ' . $url);
|
||||||
|
}
|
||||||
|
|
||||||
return $result['token'];
|
return $result['token'];
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -105,8 +105,11 @@ class OCSAuthAPITest extends TestCase {
|
||||||
if ($expected === Http::STATUS_OK) {
|
if ($expected === Http::STATUS_OK) {
|
||||||
$this->jobList->expects($this->once())->method('add')
|
$this->jobList->expects($this->once())->method('add')
|
||||||
->with('OCA\Federation\BackgroundJob\GetSharedSecret', ['url' => $url, 'token' => $token]);
|
->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 {
|
} else {
|
||||||
$this->jobList->expects($this->never())->method('add');
|
$this->jobList->expects($this->never())->method('add');
|
||||||
|
$this->jobList->expects($this->never())->method('remove');
|
||||||
}
|
}
|
||||||
|
|
||||||
$result = $this->ocsAuthApi->requestSharedSecret();
|
$result = $this->ocsAuthApi->requestSharedSecret();
|
||||||
|
|
Loading…
Reference in New Issue