fix(security): validate federation server against trusted-servers list (OC10-100)

lib/AppInfo/Application.php

@@ -14,6 +14,7 @@
 namespace OCA\Richdocuments\AppInfo;
 
 use OCA\Richdocuments\AppConfig;
+use OCA\Richdocuments\FederationService;
 use OCP\AppFramework\App;
 use OC\AppFramework\Utility\SimpleContainer;
 use OCP\Share;
@@ -36,6 +37,24 @@ private function registerServices() {
 		$container->registerService('L10N', function (SimpleContainer $c) use ($server) {
 			return $server->getL10N($c->query('AppName'));
 		});
+
+		/**
+		 * FederationService — inject TrustedServers only when the federation app is installed.
+		 * When null, FederationService::isServerAllowed() denies all remote servers (secure default).
+		 */
+		$container->registerService(FederationService::class, function () use ($server) {
+			$trustedServers = null;
+			if ($server->getAppManager()->isInstalled('federation')) {
+				/* @phan-suppress-next-line PhanUndeclaredClassReference */
+				$trustedServers = $server->query(\OCA\Federation\TrustedServers::class);
+			}
+			return new FederationService(
+				$server->getLogger(),
+				$server->getURLGenerator(),
+				$server->getHTTPClientService(),
+				$trustedServers
+			);
+		});
 	}
 
 	public function registerScripts() {

lib/FederationService.php

@@ -21,34 +21,36 @@
  */
 namespace OCA\Richdocuments;
 
+use OCA\Federation\TrustedServers;
 use OCP\ILogger;
 use OCP\Http\Client\IClientService;
 use OCP\IURLGenerator;
 
 class FederationService {
-	/**
-	 * @var ILogger
-	 */
+	/** @var ILogger */
 	private $logger;
 
-	/**
-	 * @var IURLGenerator
-	 */
+	/** @var IURLGenerator */
 	private $urlGenerator;
 
-	/**
-	 * @var IClientService
-	 */
+	/** @var IClientService */
 	private $httpClient;
 
+	/** @var TrustedServers|null */
+	/* @phan-suppress-next-line PhanUndeclaredTypeProperty */
+	private $trustedServers;
+
 	public function __construct(
 		ILogger $logger,
 		IURLGenerator $urlGenerator,
-		IClientService $httpClient
+		IClientService $httpClient,
+		/* @phan-suppress-next-line PhanUndeclaredTypeParameter */
+		?TrustedServers $trustedServers
 	) {
-		$this->logger = $logger;
-		$this->urlGenerator = $urlGenerator;
-		$this->httpClient = $httpClient;
+		$this->logger         = $logger;
+		$this->urlGenerator   = $urlGenerator;
+		$this->httpClient     = $httpClient;
+		$this->trustedServers = $trustedServers;
 	}
 
 	/**
@@ -69,13 +71,12 @@ public function getRemoteFileUrl(string $shareToken, string $shareRelativePath,
 			'&accessToken=' . $accessToken;
 		return $remoteFileUrl;
 	}
-	
+
 	/**
-	*
-	* @param string $server addres of a remote server
-	* @param string $accessToken wopi access token from a remote server
-	* @return array|null with additional wopi information
-	*/
+	 * @param string $server address of a remote server
+	 * @param string $accessToken wopi access token from a remote server
+	 * @return array|null with additional wopi information
+	 */
 	public function getWopiForToken($server, $accessToken) {
 		$remote = $server;
 
@@ -114,15 +115,39 @@ public function getWopiForToken($server, $accessToken) {
 	}
 
 	/**
-	 * Check if server is allowed
+	 * Check if the given server URL is in ownCloud's trusted-servers list.
+	 *
+	 * Returns false when the federation app is not installed ($trustedServers is null)
+	 * or when the server is not in the trusted list. Checks both the URL as-is (after
+	 * stripping trailing slashes) and the http/https scheme variant to tolerate minor
+	 * mismatches between how the admin stored the URL and how the request arrives.
 	 *
 	 * @param string $remote a remote url
-	 * @return bool indicating if given remote is allowed server
+	 * @return bool
 	 */
-	public function isServerAllowed($remote) {
-		// TODO: implement check for trusted server, for a moment all trusted
+	public function isServerAllowed(string $remote): bool {
+		if ($this->trustedServers === null) {
+			return false;
+		}
+
+		$normalized = \rtrim($remote, '/');
+
+		/* @phan-suppress-next-line PhanUndeclaredClassMethod */
+		if ($this->trustedServers->isTrustedServer($normalized)) {
+			return true;
+		}
+
+		// swap scheme and try again
+		if (\strpos($normalized, 'https://') === 0) {
+			$swapped = 'http://' . \substr($normalized, 8);
+		} elseif (\strpos($normalized, 'http://') === 0) {
+			$swapped = 'https://' . \substr($normalized, 7);
+		} else {
+			return false;
+		}
 
-		return true;
+		/* @phan-suppress-next-line PhanUndeclaredClassMethod */
+		return $this->trustedServers->isTrustedServer($swapped);
 	}
 
 	/**

tests/unit/FederationServiceTest.php

@@ -21,6 +21,7 @@
  */
 namespace OCA\Richdocuments\Tests;
 
+use OCA\Federation\TrustedServers;
 use OCA\Richdocuments\FederationService;
 use OCP\Http\Client\IClientService;
 use OCP\ILogger;
@@ -29,57 +30,44 @@
 use Test\TestCase;
 
 class FederationServiceTest extends TestCase {
-	/**
-	 * The ILogger instance.
-	 *
-	 * @var ILogger
-	 */
+	/** @var ILogger|MockObject */
 	private $logger;
 
-	/**
-	 * The IClientService instance.
-	 *
-	 * @var IClientService
-	 */
+	/** @var IClientService|MockObject */
 	private $httpClient;
 
-	/**
-	 * The IURLGenerator instance.
-	 *
-	 * @var IURLGenerator
-	 */
+	/** @var IURLGenerator|MockObject */
 	private $urlGenerator;
 
-	/**
-	 * @var FederationService|MockObject The discovery service mock object.
-	 */
+	/** @var TrustedServers|MockObject */
+	private $trustedServers;
+
+	/** @var FederationService */
 	private $federationService;
 
 	protected function setUp(): void {
 		parent::setUp();
 
-		$this->urlGenerator = $this->createMock(IURLGenerator::class);
-		$this->logger = $this->createMock(ILogger::class);
-		$this->httpClient = $this->createMock(IClientService::class);
+		$this->urlGenerator   = $this->createMock(IURLGenerator::class);
+		$this->logger         = $this->createMock(ILogger::class);
+		$this->httpClient     = $this->createMock(IClientService::class);
+		$this->trustedServers = $this->createMock(TrustedServers::class);
 
 		$this->federationService = new FederationService(
 			$this->logger,
 			$this->urlGenerator,
-			$this->httpClient
+			$this->httpClient,
+			$this->trustedServers
 		);
 	}
 
+	// -------------------------------------------------------------------------
+	// Existing tests
+	// -------------------------------------------------------------------------
+
 	public function dataGenerateFederatedCloudID() {
-		$userPrefix = [
-			'username',
-			'1234'
-		];
-		$remotes = [
-			'localhost',
-			'local.host',
-			'dev.local.host',
-			'127.0.0.1',
-		];
+		$userPrefix = ['username', '1234'];
+		$remotes    = ['localhost', 'local.host', 'dev.local.host', '127.0.0.1'];
 
 		$testCases = [];
 		foreach ($userPrefix as $user) {
@@ -92,9 +80,6 @@ public function dataGenerateFederatedCloudID() {
 
 	/**
 	 * @dataProvider dataGenerateFederatedCloudID
-	 *
-	 * @param string $userId
-	 * @param string $expectedFederatedCloudID
 	 */
 	public function testSplitUserRemote($userId, $remote) {
 		$this->urlGenerator->method('getAbsoluteUrl')
@@ -105,4 +90,64 @@ public function testSplitUserRemote($userId, $remote) {
 
 		$this->assertSame("{$userId}@{$remote}", $federatedCloudID);
 	}
+
+	// -------------------------------------------------------------------------
+	// isServerAllowed() tests
+	// -------------------------------------------------------------------------
+
+	public function testIsServerAllowedReturnsFalseWhenTrustedServersIsNull(): void {
+		$service = new FederationService(
+			$this->logger,
+			$this->urlGenerator,
+			$this->httpClient,
+			null
+		);
+
+		$this->assertFalse($service->isServerAllowed('https://remote.example.com'));
+	}
+
+	public function testIsServerAllowedReturnsTrueForExactMatch(): void {
+		$this->trustedServers->method('isTrustedServer')
+			->willReturnCallback(fn ($url) => $url === 'https://trusted.example.com');
+
+		$this->assertTrue($this->federationService->isServerAllowed('https://trusted.example.com'));
+	}
+
+	public function testIsServerAllowedStripsTrailingSlash(): void {
+		$this->trustedServers->method('isTrustedServer')
+			->willReturnCallback(fn ($url) => $url === 'https://trusted.example.com');
+
+		$this->assertTrue($this->federationService->isServerAllowed('https://trusted.example.com/'));
+	}
+
+	public function testIsServerAllowedStripsMultipleTrailingSlashes(): void {
+		$this->trustedServers->method('isTrustedServer')
+			->willReturnCallback(fn ($url) => $url === 'https://trusted.example.com');
+
+		$this->assertTrue($this->federationService->isServerAllowed('https://trusted.example.com///'));
+	}
+
+	public function testIsServerAllowedSwapsHttpToHttps(): void {
+		// Admin stored https://, request arrives as http://
+		$this->trustedServers->method('isTrustedServer')
+			->willReturnCallback(fn ($url) => $url === 'https://trusted.example.com');
+
+		$this->assertTrue($this->federationService->isServerAllowed('http://trusted.example.com'));
+	}
+
+	public function testIsServerAllowedSwapsHttpsToHttp(): void {
+		// Admin stored http://, request arrives as https://
+		$this->trustedServers->method('isTrustedServer')
+			->willReturnCallback(fn ($url) => $url === 'http://trusted.example.com');
+
+		$this->assertTrue($this->federationService->isServerAllowed('https://trusted.example.com'));
+	}
+
+	public function testIsServerAllowedReturnsFalseForUntrustedServer(): void {
+		$this->trustedServers->method('isTrustedServer')
+			->willReturn(false);
+
+		$this->assertFalse($this->federationService->isServerAllowed('https://evil.attacker.com'));
+	}
+
 }