Add error logging to federated sharing handshake

diff --git a/apps/federation/api/ocsauthapi.php b/apps/federation/api/ocsauthapi.php
index d165a0bd22fc87108c80f0a4e280d1c575fa4793..b94550fd4f2bb24b9416330af4ab42760bc6cb1b 100644 (file)
--- 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 172283536b4119d79ca5af65d1aaf8dec8544c73..45d88548b70d2919a702698b6bac3edc694d8ffe 100644 (file)
--- 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 eb55fa2d6ab183d02f8a48ba657802331bd8c85c..8aa8a08e07b9599ed16f7153bc05fdb4a9e9e29e 100644 (file)
--- 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 24d8adada1113821e91cfb729b0839ebd2670847..a1906d2082339dad60861c77cd2a81e39fcb75d9 100644 (file)
--- 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 a334686c24e424c0c98796144d3969f7baca1f6e..e6a95af8585daa23d7183cdf483c1debeaba278f 100644 (file)
--- 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();