Forward exception message to the admin in case of errors and in case the remote server version is to low and appropriate message is displayed as well

5 files changed, 46 insertions, 36 deletions

diff --git a/apps/federation/controller/settingscontroller.php b/apps/federation/controller/settingscontroller.php
index e5e46606f12..3adb6fced66 100644
--- a/apps/federation/controller/settingscontroller.php
+++ b/apps/federation/controller/settingscontroller.php
@@ -26,7 +26,6 @@ use OCA\Federation\TrustedServers;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\DataResponse;
-use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IRequest;
 
diff --git a/apps/federation/lib/trustedservers.php b/apps/federation/lib/trustedservers.php
index e3ce8228cc3..340accfdbdf 100644
--- a/apps/federation/lib/trustedservers.php
+++ b/apps/federation/lib/trustedservers.php
@@ -23,6 +23,7 @@
 
 namespace OCA\Federation;
 
+use OC\HintException;
 use OCP\AppFramework\Http;
 use OCP\BackgroundJob\IJobList;
 use OCP\Http\Client\IClientService;
@@ -202,34 +203,33 @@ class TrustedServers {
 	public function isOwnCloudServer($url) {
 		$isValidOwnCloud = false;
 		$client = $this->httpClientService->newClient();
-		try {
-			$result = $client->get(
-				$url . '/status.php',
-				[
-					'timeout' => 3,
-					'connect_timeout' => 3,
-				]
-			);
-			if ($result->getStatusCode() === Http::STATUS_OK) {
-				$isValidOwnCloud = $this->checkOwnCloudVersion($result->getBody());
-			}
-		} catch (\Exception $e) {
-			$this->logger->error($e->getMessage(), ['app' => 'federation']);
-			return false;
+		$result = $client->get(
+			$url . '/status.php',
+			[
+				'timeout' => 3,
+				'connect_timeout' => 3,
+			]
+		);
+		if ($result->getStatusCode() === Http::STATUS_OK) {
+			$isValidOwnCloud = $this->checkOwnCloudVersion($result->getBody());
 		}
+
 		return $isValidOwnCloud;
 	}
 
 	/**
 	 * check if ownCloud version is >= 9.0
 	 *
-	 * @param $statusphp
+	 * @param $status
 	 * @return bool
 	 */
-	protected function checkOwnCloudVersion($statusphp) {
-		$decoded = json_decode($statusphp, true);
+	protected function checkOwnCloudVersion($status) {
+		$decoded = json_decode($status, true);
 		if (!empty($decoded) && isset($decoded['version'])) {
-			return version_compare($decoded['version'], '9.0.0', '>=');
+			if (!version_compare($decoded['version'], '9.0.0', '>=')) {
+				throw new HintException('Remote server version is too low. ownCloud 9.0 is required.');
+			}
+			return true;
 		}
 		return false;
 	}
diff --git a/apps/federation/middleware/addservermiddleware.php b/apps/federation/middleware/addservermiddleware.php
index cd9ccff4403..10abd9db704 100644
--- a/apps/federation/middleware/addservermiddleware.php
+++ b/apps/federation/middleware/addservermiddleware.php
@@ -58,7 +58,7 @@ class AddServerMiddleware extends Middleware {
 		if ($exception instanceof HintException) {
 			$message = $exception->getHint();
 		} else {
-			$message = $this->l->t('Unknown error');
+			$message = $exception->getMessage();
 		}
 
 		return new JSONResponse(
diff --git a/apps/federation/tests/lib/trustedserverstest.php b/apps/federation/tests/lib/trustedserverstest.php
index e57391ed198..130a0e3bb22 100644
--- a/apps/federation/tests/lib/trustedserverstest.php
+++ b/apps/federation/tests/lib/trustedserverstest.php
@@ -23,6 +23,7 @@
 namespace OCA\Federation\Tests\lib;
 
 
+use OC\HintException;
 use OCA\Federation\DbHandler;
 use OCA\Federation\TrustedServers;
 use OCP\BackgroundJob\IJobList;
@@ -282,41 +283,50 @@ class TrustedServersTest extends TestCase {
 		];
 	}
 
+	/**
+	 * @expectedException \Exception
+	 * @expectedExceptionMessage simulated exception
+	 */
 	public function testIsOwnCloudServerFail() {
 		$server = 'server1';
 
 		$this->httpClientService->expects($this->once())->method('newClient')
 			->willReturn($this->httpClient);
 
-		$this->logger->expects($this->once())->method('error')
-			->with('simulated exception', ['app' => 'federation']);
-
 		$this->httpClient->expects($this->once())->method('get')->with($server . '/status.php')
 			->willReturnCallback(function () {
 				throw new \Exception('simulated exception');
 			});
 
-		$this->assertFalse($this->trustedServers->isOwnCloudServer($server));
-
+		$this->trustedServers->isOwnCloudServer($server);
 	}
 
 	/**
 	 * @dataProvider dataTestCheckOwnCloudVersion
-	 *
-	 * @param $statusphp
-	 * @param $expected
 	 */
-	public function testCheckOwnCloudVersion($statusphp, $expected) {
-		$this->assertSame($expected,
-			$this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$statusphp])
-		);
+	public function testCheckOwnCloudVersion($status) {
+		$this->assertTrue($this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$status]));
 	}
 
 	public function dataTestCheckOwnCloudVersion() {
 		return [
-			['{"version":"8.4.0"}', false],
-			['{"version":"9.0.0"}', true],
-			['{"version":"9.1.0"}', true]
+			['{"version":"9.0.0"}'],
+			['{"version":"9.1.0"}']
+		];
+	}
+
+	/**
+	 * @dataProvider dataTestCheckOwnCloudVersionTooLow
+	 * @expectedException \OC\HintException
+	 * @expectedExceptionMessage Remote server version is too low. ownCloud 9.0 is required.
+	 */
+	public function testCheckOwnCloudVersionTooLow($status) {
+		$this->invokePrivate($this->trustedServers, 'checkOwnCloudVersion', [$status]);
+	}
+
+	public function dataTestCheckOwnCloudVersionTooLow() {
+		return [
+			['{"version":"8.2.3"}'],
 		];
 	}
 
diff --git a/apps/federation/tests/middleware/addservermiddlewaretest.php b/apps/federation/tests/middleware/addservermiddlewaretest.php
index a94d907ae76..49e34cc73d6 100644
--- a/apps/federation/tests/middleware/addservermiddlewaretest.php
+++ b/apps/federation/tests/middleware/addservermiddlewaretest.php
@@ -27,6 +27,7 @@ use OC\HintException;
 use OCA\Federation\Middleware\AddServerMiddleware;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\ILogger;
 use Test\TestCase;
 
 class AddServerMiddlewareTest extends TestCase {
@@ -93,7 +94,7 @@ class AddServerMiddlewareTest extends TestCase {
 	public function dataTestAfterException() {
 		return [
 			[new HintException('message', 'hint'), 'message', 'hint'],
-			[new \Exception('message'), 'message', 'Unknown error'],
+			[new \Exception('message'), 'message', 'message'],
 		];
 	}