Commit 152e72c4 authored by Thomas Müller's avatar Thomas Müller
Browse files

Merge pull request #21312 from owncloud/fed-handshake-logging

Add error logging to federation handshake
parents 6b1cb44c ef9a231f
......@@ -26,6 +26,7 @@ use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http;
use OCP\BackgroundJob\IJobList;
use OCP\ILogger;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
use OCP\Security\StringUtils;
......@@ -54,6 +55,9 @@ class OCSAuthAPI {
/** @var DbHandler */
private $dbHandler;
/** @var ILogger */
private $logger;
/**
* OCSAuthAPI constructor.
*
......@@ -62,19 +66,22 @@ class OCSAuthAPI {
* @param IJobList $jobList
* @param TrustedServers $trustedServers
* @param DbHandler $dbHandler
* @param ILogger $logger
*/
public function __construct(
IRequest $request,
ISecureRandom $secureRandom,
IJobList $jobList,
TrustedServers $trustedServers,
DbHandler $dbHandler
DbHandler $dbHandler,
ILogger $logger
) {
$this->request = $request;
$this->secureRandom = $secureRandom;
$this->jobList = $jobList;
$this->trustedServers = $trustedServers;
$this->dbHandler = $dbHandler;
$this->logger = $logger;
}
/**
......@@ -88,6 +95,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');
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
}
......@@ -95,6 +103,7 @@ 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');
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
}
......@@ -120,10 +129,13 @@ class OCSAuthAPI {
$url = $this->request->getParam('url');
$token = $this->request->getParam('token');
if (
$this->trustedServers->isTrustedServer($url) === false
|| $this->isValidToken($url, $token) === false
) {
if ($this->trustedServers->isTrustedServer($url) === false) {
$this->logger->log(\OCP\Util::ERROR, 'remote server not trusted (' . $url . ') while getting shared secret');
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');
return new \OC_OCS_Result(null, HTTP::STATUS_FORBIDDEN);
}
......
......@@ -108,7 +108,8 @@ class Application extends \OCP\AppFramework\App {
$server->getSecureRandom(),
$server->getJobList(),
$container->query('TrustedServers'),
$container->query('DbHandler')
$container->query('DbHandler'),
$server->getLogger()
);
......
......@@ -91,7 +91,7 @@ class GetSharedSecret extends QueuedJob{
$this->trustedServers = new TrustedServers(
$this->dbHandler,
\OC::$server->getHTTPClientService(),
\OC::$server->getLogger(),
$this->logger,
$this->jobList,
\OC::$server->getSecureRandom(),
\OC::$server->getConfig()
......@@ -148,6 +148,7 @@ class GetSharedSecret extends QueuedJob{
} catch (ClientException $e) {
$status = $e->getCode();
$this->logger->logException($e);
}
// if we received a unexpected response we try again later
......
......@@ -60,6 +60,9 @@ class RequestSharedSecret extends QueuedJob {
private $endPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret?format=json';
/** @var ILogger */
private $logger;
/**
* RequestSharedSecret constructor.
*
......@@ -80,13 +83,14 @@ class RequestSharedSecret extends QueuedJob {
$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
$this->logger = \OC::$server->getLogger();
if ($trustedServers) {
$this->trustedServers = $trustedServers;
} else {
$this->trustedServers = new TrustedServers(
$this->dbHandler,
\OC::$server->getHTTPClientService(),
\OC::$server->getLogger(),
$this->logger,
$this->jobList,
\OC::$server->getSecureRandom(),
\OC::$server->getConfig()
......@@ -142,6 +146,7 @@ class RequestSharedSecret extends QueuedJob {
} catch (ClientException $e) {
$status = $e->getCode();
$this->logger->logException($e);
}
// if we received a unexpected response we try again later
......
......@@ -28,6 +28,7 @@ use OCA\Federation\API\OCSAuthAPI;
use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http;
use OCP\ILogger;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
use Test\TestCase;
......@@ -49,6 +50,9 @@ class OCSAuthAPITest extends TestCase {
/** @var \PHPUnit_Framework_MockObject_MockObject | DbHandler */
private $dbHandler;
/** @var \PHPUnit_Framework_MockObject_MockObject | ILogger */
private $logger;
/** @var OCSAuthApi */
private $ocsAuthApi;
......@@ -63,13 +67,16 @@ class OCSAuthAPITest extends TestCase {
->disableOriginalConstructor()->getMock();
$this->jobList = $this->getMockBuilder('OC\BackgroundJob\JobList')
->disableOriginalConstructor()->getMock();
$this->logger = $this->getMockBuilder('OCP\ILogger')
->disableOriginalConstructor()->getMock();
$this->ocsAuthApi = new OCSAuthAPI(
$this->request,
$this->secureRandom,
$this->jobList,
$this->trustedServers,
$this->dbHandler
$this->dbHandler,
$this->logger
);
}
......@@ -136,7 +143,8 @@ class OCSAuthAPITest extends TestCase {
$this->secureRandom,
$this->jobList,
$this->trustedServers,
$this->dbHandler
$this->dbHandler,
$this->logger
]
)->setMethods(['isValidToken'])->getMock();
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment