Skip to content

Commit 399f04f

Browse files
committed
fix: don't fetch own remote collabora url
Signed-off-by: Elizabeth Danzberger <lizzy7128@tutanota.de>
1 parent e364a5e commit 399f04f

2 files changed

Lines changed: 96 additions & 0 deletions

File tree

lib/Service/FederationService.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use OCP\ICacheFactory;
2525
use OCP\IRequest;
2626
use OCP\IURLGenerator;
27+
use OCP\Security\ITrustedDomainHelper;
2728
use OCP\Share\IShare;
2829
use Psr\Container\ContainerExceptionInterface;
2930
use Psr\Container\NotFoundExceptionInterface;
@@ -43,6 +44,7 @@ public function __construct(
4344
private AppConfig $appConfig,
4445
private IRequest $request,
4546
private IURLGenerator $urlGenerator,
47+
private ITrustedDomainHelper $trustedDomainHelper,
4648
) {
4749
$this->cache = $cacheFactory->createDistributed('richdocuments_remote/');
4850
try {
@@ -73,6 +75,11 @@ public function getRemoteCollaboraURL($remote) {
7375
if (!$this->isTrustedRemote($remote)) {
7476
throw new \Exception('Unable to determine collabora URL of remote server ' . $remote . ' - Remote is not a trusted server');
7577
}
78+
79+
if ($this->trustedDomainHelper->isTrustedUrl($remote)) {
80+
return $this->appConfig->getCollaboraUrlInternal();
81+
}
82+
7683
$remoteCollabora = $this->cache->get('richdocuments_remote/' . $remote);
7784
if ($remoteCollabora !== null) {
7885
return $remoteCollabora;
@@ -112,7 +119,12 @@ public function isTrustedRemote($domainWithPort) {
112119
if (!is_string($trusted)) {
113120
break;
114121
}
122+
123+
// This regular expression ensures that wildcards for trusted domains
124+
// are parsed properly in order to match subdomains:
125+
// *.example.com => /^[-\.a-zA-Z0-9]*\.example\.com$/i
115126
$regex = '/^' . implode('[-\.a-zA-Z0-9]*', array_map(fn ($v) => preg_quote($v, '/'), explode('*', $trusted))) . '$/i';
127+
116128
if (preg_match($regex, $domain) || preg_match($regex, $domainWithPort)) {
117129
return true;
118130
}
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
declare(strict_types = 1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace Tests\Richdocuments;
11+
12+
use OCA\Federation\TrustedServers;
13+
use OCA\Richdocuments\AppConfig;
14+
use OCA\Richdocuments\Service\FederationService;
15+
use OCA\Richdocuments\TokenManager;
16+
use OCP\Http\Client\IClientService;
17+
use OCP\ICacheFactory;
18+
use OCP\IRequest;
19+
use OCP\IURLGenerator;
20+
use OCP\Security\ITrustedDomainHelper;
21+
use PHPUnit\Framework\TestCase;
22+
use Psr\Log\LoggerInterface;
23+
24+
class FederationServiceTest extends TestCase {
25+
public const NEXTCLOUD_ADDRESS = 'https://nextcloud.local';
26+
public const COLLABORA_ADDRESS = 'https://collabora.local';
27+
28+
public function setUp(): void {
29+
parent::setUp();
30+
31+
$this->cacheFactory = $this->createMock(ICacheFactory::class);
32+
$this->clientService = $this->createMock(IClientService::class);
33+
$this->logger = $this->createMock(LoggerInterface::class);
34+
$this->tokenManager = $this->createMock(TokenManager::class);
35+
$this->appConfig = $this->createStub(AppConfig::class);
36+
$this->request = $this->createMock(IRequest::class);
37+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
38+
$this->trustedDomainHelper = $this->createStub(ITrustedDomainHelper::class);
39+
40+
$this->federationService = new FederationService(
41+
$this->cacheFactory,
42+
$this->clientService,
43+
$this->logger,
44+
$this->tokenManager,
45+
$this->appConfig,
46+
$this->request,
47+
$this->urlGenerator,
48+
$this->trustedDomainHelper
49+
);
50+
}
51+
52+
/**
53+
* @test
54+
* @testdox returns own instance's Collabora URL
55+
*/
56+
public function getRemoteCollaboraURLFromOwnInstance(): void {
57+
// Ensure that trusted domains can be used for federated editing
58+
$this->appConfig->method('isTrustedDomainAllowedForFederation')
59+
->willReturn(true);
60+
$this->appConfig->method('getCollaboraUrlInternal')
61+
->willReturn(self::COLLABORA_ADDRESS);
62+
63+
$this->trustedDomainHelper->method('isTrustedUrl')
64+
->with(self::NEXTCLOUD_ADDRESS)
65+
->willReturn(true);
66+
67+
// Create a stub TrustedServers class which always tells us
68+
// the server is trusted
69+
$trustedServers = $this->createStub(TrustedServers::class);
70+
$trustedServers->method('isTrustedServer')
71+
->with('nextcloud.local')
72+
->willReturn(true);
73+
74+
// Do some reflection property manipulation to set the TrustedServers object
75+
// It would be nice if the TrustedServers were passed into FederationService
76+
// instead of being set manually in the constructor to make testing easier
77+
$reflection = new \ReflectionClass($this->federationService);
78+
$reflectionProperty = $reflection->getProperty('trustedServers');
79+
$reflectionProperty->setAccessible(true);
80+
$reflectionProperty->setValue($this->federationService, $trustedServers);
81+
82+
$this->assertEquals(self::COLLABORA_ADDRESS, $this->federationService->getRemoteCollaboraURL(self::NEXTCLOUD_ADDRESS));
83+
}
84+
}

0 commit comments

Comments
 (0)