From 2c2e1c4c7a94cdd3319c029e22e616df0853310e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 11 Dec 2017 15:07:08 +0100 Subject: [PATCH] Catch the errors related to untrusted self signed certificates for federation * Added tests Signed-off-by: Roeland Jago Douma --- .../lib/BackgroundJob/GetSharedSecret.php | 7 +++- .../lib/BackgroundJob/RequestSharedSecret.php | 7 +++- .../BackgroundJob/GetSharedSecretTest.php | 38 +++++++++++++++++++ .../BackgroundJob/RequestSharedSecretTest.php | 38 +++++++++++++++++++ 4 files changed, 88 insertions(+), 2 deletions(-) diff --git a/apps/federation/lib/BackgroundJob/GetSharedSecret.php b/apps/federation/lib/BackgroundJob/GetSharedSecret.php index 6090f521fc..92bb31e369 100644 --- a/apps/federation/lib/BackgroundJob/GetSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/GetSharedSecret.php @@ -32,6 +32,8 @@ namespace OCA\Federation\BackgroundJob; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Ring\Exception\RingException; use OC\BackgroundJob\JobList; use OC\BackgroundJob\Job; use OCA\Federation\DbHandler; @@ -197,7 +199,10 @@ class GetSharedSecret extends Job { } else { $this->logger->info($target . ' responded with a ' . $status . ' containing: ' . $e->getMessage(), ['app' => 'federation']); } - } catch (ConnectException $e) { + } catch (RequestException $e) { + $status = -1; // There is no status code if we could not connect + $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); + } catch (RingException $e) { $status = -1; // There is no status code if we could not connect $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); } catch (\Exception $e) { diff --git a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php index a201c9dccb..ad7504da7a 100644 --- a/apps/federation/lib/BackgroundJob/RequestSharedSecret.php +++ b/apps/federation/lib/BackgroundJob/RequestSharedSecret.php @@ -33,6 +33,8 @@ namespace OCA\Federation\BackgroundJob; use GuzzleHttp\Exception\ClientException; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Ring\Exception\RingException; use OC\BackgroundJob\JobList; use OC\BackgroundJob\Job; use OCA\Federation\DbHandler; @@ -197,7 +199,10 @@ class RequestSharedSecret extends Job { } else { $this->logger->info($target . ' responded with a ' . $status . ' containing: ' . $e->getMessage(), ['app' => 'federation']); } - } catch (ConnectException $e) { + } catch (RequestException $e) { + $status = -1; // There is no status code if we could not connect + $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); + } catch (RingException $e) { $status = -1; // There is no status code if we could not connect $this->logger->info('Could not connect to ' . $target, ['app' => 'federation']); } catch (\Exception $e) { diff --git a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php index 2058b2592c..1e264919e7 100644 --- a/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php +++ b/apps/federation/tests/BackgroundJob/GetSharedSecretTest.php @@ -29,6 +29,7 @@ namespace OCA\Federation\Tests\BackgroundJob; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Ring\Exception\RingException; use OCA\Federation\BackgroundJob\GetSharedSecret; use OCA\Files_Sharing\Tests\TestCase; use OCA\Federation\DbHandler; @@ -315,4 +316,41 @@ class GetSharedSecretTest extends TestCase { $this->assertTrue($this->invokePrivate($this->getSharedSecret, 'retainJob')); } + + public function testRunRingException() { + $target = 'targetURL'; + $source = 'sourceURL'; + $token = 'token'; + + $argument = ['url' => $target, 'token' => $token]; + + $this->timeFactory->method('getTime') + ->willReturn(42); + + $this->urlGenerator + ->expects($this->once()) + ->method('getAbsoluteURL') + ->with('/') + ->willReturn($source); + $this->httpClient->expects($this->once())->method('get') + ->with( + $target . '/ocs/v2.php/apps/federation/api/v1/shared-secret?format=json', + [ + 'query' => + [ + 'url' => $source, + 'token' => $token + ], + 'timeout' => 3, + 'connect_timeout' => 3, + ] + )->willThrowException($this->createMock(RingException::class)); + + $this->dbHandler->expects($this->never())->method('addToken'); + $this->trustedServers->expects($this->never())->method('addSharedSecret'); + + $this->invokePrivate($this->getSharedSecret, 'run', [$argument]); + + $this->assertTrue($this->invokePrivate($this->getSharedSecret, 'retainJob')); + } } diff --git a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php index 57a85f1be0..20610f1f0f 100644 --- a/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php +++ b/apps/federation/tests/BackgroundJob/RequestSharedSecretTest.php @@ -28,6 +28,7 @@ namespace OCA\Federation\Tests\BackgroundJob; use GuzzleHttp\Exception\ConnectException; +use GuzzleHttp\Ring\Exception\RingException; use OCA\Federation\BackgroundJob\RequestSharedSecret; use OCA\Federation\DbHandler; use OCA\Federation\TrustedServers; @@ -300,4 +301,41 @@ class RequestSharedSecretTest extends TestCase { $this->invokePrivate($this->requestSharedSecret, 'run', [$argument]); $this->assertTrue($this->invokePrivate($this->requestSharedSecret, 'retainJob')); } + + public function testRunRingException() { + $target = 'targetURL'; + $source = 'sourceURL'; + $token = 'token'; + + $argument = ['url' => $target, 'token' => $token]; + + $this->timeFactory->method('getTime')->willReturn(42); + + $this->urlGenerator + ->expects($this->once()) + ->method('getAbsoluteURL') + ->with('/') + ->willReturn($source); + + $this->httpClient + ->expects($this->once()) + ->method('post') + ->with( + $target . '/ocs/v2.php/apps/federation/api/v1/request-shared-secret?format=json', + [ + 'body' => + [ + 'url' => $source, + 'token' => $token + ], + 'timeout' => 3, + 'connect_timeout' => 3, + ] + )->willThrowException($this->createMock(RingException::class)); + + $this->dbHandler->expects($this->never())->method('addToken'); + + $this->invokePrivate($this->requestSharedSecret, 'run', [$argument]); + $this->assertTrue($this->invokePrivate($this->requestSharedSecret, 'retainJob')); + } }