diff options
author | Robin Appelman <icewind@owncloud.com> | 2015-12-21 17:48:02 +0300 |
---|---|---|
committer | Robin Appelman <icewind@owncloud.com> | 2015-12-21 18:37:54 +0300 |
commit | ef9a231f6cafde960aaa281c5ef6c3ef4091f35f (patch) | |
tree | c43fdc0391e85d0131a1bbc7eeef6e47d820a9ee /apps | |
parent | 38a006b1e845de399ed10001d3ff9efbe76a5de5 (diff) |
Add error logging to federated sharing handshake
Diffstat (limited to 'apps')
-rw-r--r-- | apps/federation/api/ocsauthapi.php | 22 | ||||
-rw-r--r-- | apps/federation/appinfo/application.php | 3 | ||||
-rw-r--r-- | apps/federation/backgroundjob/getsharedsecret.php | 3 | ||||
-rw-r--r-- | apps/federation/backgroundjob/requestsharedsecret.php | 7 | ||||
-rw-r--r-- | apps/federation/tests/api/ocsauthapitest.php | 12 |
5 files changed, 37 insertions, 10 deletions
diff --git a/apps/federation/api/ocsauthapi.php b/apps/federation/api/ocsauthapi.php index d165a0bd22f..b94550fd4f2 100644 --- a/apps/federation/api/ocsauthapi.php +++ b/apps/federation/api/ocsauthapi.php @@ -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); } diff --git a/apps/federation/appinfo/application.php b/apps/federation/appinfo/application.php index 172283536b4..45d88548b70 100644 --- a/apps/federation/appinfo/application.php +++ b/apps/federation/appinfo/application.php @@ -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() ); diff --git a/apps/federation/backgroundjob/getsharedsecret.php b/apps/federation/backgroundjob/getsharedsecret.php index eb55fa2d6ab..8aa8a08e07b 100644 --- a/apps/federation/backgroundjob/getsharedsecret.php +++ b/apps/federation/backgroundjob/getsharedsecret.php @@ -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 diff --git a/apps/federation/backgroundjob/requestsharedsecret.php b/apps/federation/backgroundjob/requestsharedsecret.php index 24d8adada11..a1906d20823 100644 --- a/apps/federation/backgroundjob/requestsharedsecret.php +++ b/apps/federation/backgroundjob/requestsharedsecret.php @@ -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 diff --git a/apps/federation/tests/api/ocsauthapitest.php b/apps/federation/tests/api/ocsauthapitest.php index a334686c24e..e6a95af8585 100644 --- a/apps/federation/tests/api/ocsauthapitest.php +++ b/apps/federation/tests/api/ocsauthapitest.php @@ -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(); |