Skip to content

Commit 39d66ed

Browse files
susnuxkesselb
susnux
and
kesselb
committed
fix(setup-checks): Ensure URL with webroot works
We basically mock the way `URLGenerator::getAbsoluteURL` works, so we must make sure that the URL might already contain the webroot. Because `baseURL` and `cliURL` also contain the webroot we need to remove the webroot from the URL first. Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de> Co-authored-by: Daniel <mail@danielkesselberg.de> Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 7e2b9a0 commit 39d66ed

7 files changed

Lines changed: 284 additions & 28 deletions

File tree

‎apps/settings/lib/SetupChecks/CheckServerResponseTrait.php‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -55,66 +55,67 @@ protected function serverConfigHelp(): string {
5555
* This takes all `trusted_domains` and the CLI overwrite URL into account.
5656
*
5757
* @param string $url The relative URL to test starting with a /
58-
* @return string[] List of possible absolute URLs
58+
* @return list<string> List of possible absolute URLs
5959
*/
6060
protected function getTestUrls(string $url, bool $removeWebroot): array {
61-
$testUrls = [];
61+
$url = '/' . ltrim($url, '/');
6262

6363
$webroot = rtrim($this->urlGenerator->getWebroot(), '/');
64+
// Similar to `getAbsoluteURL` of URLGenerator:
65+
// The Nextcloud web root could already be prepended.
66+
if ($webroot !== '' && str_starts_with($url, $webroot)) {
67+
$url = substr($url, strlen($webroot));
68+
}
69+
70+
$hosts = [];
6471

6572
/* Try overwrite.cli.url first, it’s supposed to be how the server contacts itself */
6673
$cliUrl = $this->config->getSystemValueString('overwrite.cli.url', '');
67-
6874
if ($cliUrl !== '') {
69-
$cliUrl = $this->normalizeUrl(
75+
$hosts[] = $this->normalizeUrl(
7076
$cliUrl,
7177
$webroot,
7278
$removeWebroot
7379
);
74-
75-
$testUrls[] = $cliUrl . $url;
7680
}
7781

7882
/* Try URL generator second */
79-
$baseUrl = $this->normalizeUrl(
83+
$hosts[] = $this->normalizeUrl(
8084
$this->urlGenerator->getBaseUrl(),
8185
$webroot,
8286
$removeWebroot
8387
);
8488

85-
if ($baseUrl !== $cliUrl) {
86-
$testUrls[] = $baseUrl . $url;
87-
}
88-
8989
/* Last resort: trusted domains */
90-
$hosts = $this->config->getSystemValue('trusted_domains', []);
91-
foreach ($hosts as $host) {
90+
$trustedDomains = $this->config->getSystemValue('trusted_domains', []);
91+
foreach ($trustedDomains as $host) {
9292
if (str_contains($host, '*')) {
9393
/* Ignore domains with a wildcard */
9494
continue;
9595
}
96-
$hosts[] = 'https://' . $host . $url;
97-
$hosts[] = 'http://' . $host . $url;
96+
$hosts[] = $this->normalizeUrl("https://$host$webroot", $webroot, $removeWebroot);
97+
$hosts[] = $this->normalizeUrl("http://$host$webroot", $webroot, $removeWebroot);
9898
}
9999

100-
return $testUrls;
100+
return array_map(fn (string $host) => $host . $url, array_values(array_unique($hosts)));
101101
}
102102

103103
/**
104104
* Strip a trailing slash and remove the webroot if requested.
105105
*/
106106
protected function normalizeUrl(string $url, string $webroot, bool $removeWebroot): string {
107107
$url = rtrim($url, '/');
108-
if ($removeWebroot && str_ends_with($url, $webroot)) {
109-
$url = substr($url, -strlen($webroot));
108+
if ($removeWebroot && $webroot !== '' && str_ends_with($url, $webroot)) {
109+
$url = substr($url, 0, -strlen($webroot));
110110
}
111111
return rtrim($url, '/');
112112
}
113113

114114
/**
115115
* Run a HTTP request to check header
116116
* @param string $method The HTTP method to use
117-
* @param string $url The relative URL to check
117+
* @param string $url The relative URL to check (e.g. output of IURLGenerator)
118+
* @param bool $removeWebroot Remove the webroot from the URL (handle URL as relative to domain root)
118119
* @param array{ignoreSSL?: bool, httpErrors?: bool, options?: array} $options Additional options, like
119120
* [
120121
* // Ignore invalid SSL certificates (e.g. self signed)
@@ -143,13 +144,14 @@ protected function runRequest(string $method, string $url, array $options = [],
143144

144145
/**
145146
* Run a HEAD request to check header
146-
* @param string $url The relative URL to check
147+
* @param string $url The relative URL to check (e.g. output of IURLGenerator)
147148
* @param bool $ignoreSSL Ignore SSL certificates
148149
* @param bool $httpErrors Ignore requests with HTTP errors (will not yield if request has a 4xx or 5xx response)
150+
* @param bool $removeWebroot Remove the webroot from the URL (handle URL as relative to domain root)
149151
* @return Generator<int, IResponse>
150152
*/
151-
protected function runHEAD(string $url, bool $ignoreSSL = true, bool $httpErrors = true): Generator {
152-
return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors]);
153+
protected function runHEAD(string $url, bool $ignoreSSL = true, bool $httpErrors = true, bool $removeWebroot = false): Generator {
154+
return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors], $removeWebroot);
153155
}
154156

155157
protected function getRequestOptions(bool $ignoreSSL, bool $httpErrors): array {

‎apps/settings/lib/SetupChecks/DataDirectoryProtected.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ public function getName(): string {
5858

5959
public function run(): SetupResult {
6060
$datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
61-
62-
$dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ocdata';
61+
$dataUrl = '/' . $datadir . '/.ocdata';
6362

6463
$noResponse = true;
6564
foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) {

‎apps/settings/lib/SetupChecks/WellKnownUrls.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ public function run(): SetupResult {
6868
['propfind', '/.well-known/carddav', [207], false],
6969
];
7070

71+
$requestOptions = ['httpErrors' => false, 'options' => ['allow_redirects' => ['track_redirects' => true]]];
7172
foreach ($urls as [$verb,$url,$validStatuses,$checkCustomHeader]) {
7273
$works = null;
73-
foreach ($this->runRequest($verb, $url, ['httpErrors' => false, 'options' => ['allow_redirects' => ['track_redirects' => true]]], removeWebroot: true) as $response) {
74+
foreach ($this->runRequest($verb, $url, $requestOptions, removeWebroot: true) as $response) {
7475
// Check that the response status matches
7576
$works = in_array($response->getStatusCode(), $validStatuses);
7677
// and (if needed) the custom Nextcloud header is set

‎apps/settings/tests/SetupChecks/CheckServerResponseTraitImplementation.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\Settings\Tests\SetupChecks;
10+
11+
use OCA\Settings\SetupChecks\CheckServerResponseTrait;
12+
use OCP\Http\Client\IClientService;
13+
use OCP\IConfig;
14+
use OCP\IL10N;
15+
use OCP\IURLGenerator;
16+
use Psr\Log\LoggerInterface;
17+
18+
/**
19+
* Dummy implementation for CheckServerResponseTraitTest
20+
*/
21+
class CheckServerResponseTraitImplementation {
22+
23+
use CheckServerResponseTrait {
24+
CheckServerResponseTrait::getRequestOptions as public;
25+
CheckServerResponseTrait::runHEAD as public;
26+
CheckServerResponseTrait::runRequest as public;
27+
CheckServerResponseTrait::normalizeUrl as public;
28+
CheckServerResponseTrait::getTestUrls as public;
29+
}
30+
31+
public function __construct(
32+
protected IL10N $l10n,
33+
protected IConfig $config,
34+
protected IURLGenerator $urlGenerator,
35+
protected IClientService $clientService,
36+
protected LoggerInterface $logger,
37+
) {
38+
}
39+
40+
}

‎apps/settings/tests/SetupChecks/CheckServerResponseTraitTest.php‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
namespace OCA\Settings\Tests\SetupChecks;
10+
11+
use OCP\Http\Client\IClientService;
12+
use OCP\IConfig;
13+
use OCP\IL10N;
14+
use OCP\IURLGenerator;
15+
use PHPUnit\Framework\MockObject\MockObject;
16+
use Psr\Log\LoggerInterface;
17+
use Test\TestCase;
18+
19+
class CheckServerResponseTraitTest extends TestCase {
20+
21+
protected const BASE_URL = 'https://nextcloud.local';
22+
23+
private IL10N&MockObject $l10n;
24+
private IConfig&MockObject $config;
25+
private IURLGenerator&MockObject $urlGenerator;
26+
private IClientService&MockObject $clientService;
27+
private LoggerInterface&MockObject $logger;
28+
29+
private CheckServerResponseTraitImplementation $trait;
30+
31+
protected function setUp(): void {
32+
parent::setUp();
33+
34+
$this->l10n = $this->createMock(IL10N::class);
35+
$this->l10n->method('t')
36+
->willReturnArgument(0);
37+
$this->config = $this->createMock(IConfig::class);
38+
$this->urlGenerator = $this->createMock(IURLGenerator::class);
39+
$this->clientService = $this->createMock(IClientService::class);
40+
$this->logger = $this->createMock(LoggerInterface::class);
41+
42+
$this->trait = new CheckServerResponseTraitImplementation(
43+
$this->l10n,
44+
$this->config,
45+
$this->urlGenerator,
46+
$this->clientService,
47+
$this->logger,
48+
);
49+
}
50+
51+
/**
52+
* @dataProvider dataNormalizeUrl
53+
*/
54+
public function testNormalizeUrl(string $url, string $webRoot, bool $removeWebRoot, string $expected): void {
55+
$this->assertEquals($expected, $this->trait->normalizeUrl($url, $webRoot, $removeWebRoot));
56+
}
57+
58+
public static function dataNormalizeUrl(): array {
59+
return [
60+
'valid and nothing to change' => ['http://example.com/root', '/root', false, 'http://example.com/root'],
61+
'trailing slash' => ['http://example.com/root/', '/root', false, 'http://example.com/root'],
62+
'remove web root' => ['http://example.com/root/', '/root', true, 'http://example.com'],
63+
'remove web root but empty' => ['http://example.com', '', true, 'http://example.com'],
64+
];
65+
}
66+
67+
/**
68+
* @dataProvider dataGetTestUrls
69+
*/
70+
public function testGetTestUrls(
71+
string $url,
72+
bool $removeWebRoot,
73+
string $cliUrl,
74+
string $webRoot,
75+
array $trustedDomains,
76+
array $expected,
77+
): void {
78+
$this->config->expects(self::atLeastOnce())
79+
->method('getSystemValueString')
80+
->with('overwrite.cli.url', '')
81+
->willReturn($cliUrl);
82+
83+
$this->config->expects(self::atLeastOnce())
84+
->method('getSystemValue')
85+
->with('trusted_domains', [])
86+
->willReturn($trustedDomains);
87+
88+
$this->urlGenerator->expects(self::atLeastOnce())
89+
->method('getWebroot')
90+
->willReturn($webRoot);
91+
92+
$this->urlGenerator->expects(self::atLeastOnce())
93+
->method('getBaseUrl')
94+
->willReturn(self::BASE_URL . $webRoot);
95+
96+
$result = $this->trait->getTestUrls($url, $removeWebRoot);
97+
$this->assertEquals($expected, $result);
98+
}
99+
100+
public static function dataGetTestUrls(): array {
101+
return [
102+
'same cli and base URL' => [
103+
'/apps/files/js/example.js', false, 'https://nextcloud.local', '', ['nextcloud.local'], [
104+
// from cli url
105+
'https://nextcloud.local/apps/files/js/example.js',
106+
// http variant from trusted domains
107+
'http://nextcloud.local/apps/files/js/example.js',
108+
]
109+
],
110+
'different cli and base URL' => [
111+
'/apps/files/js/example.js', false, 'https://example.com', '', ['nextcloud.local'], [
112+
// from cli url
113+
'https://example.com/apps/files/js/example.js',
114+
// from base url
115+
'https://nextcloud.local/apps/files/js/example.js',
116+
// http variant from trusted domains
117+
'http://nextcloud.local/apps/files/js/example.js',
118+
]
119+
],
120+
'different cli and base URL and trusted domains' => [
121+
'/apps/files/js/example.js', false, 'https://example.com', '', ['nextcloud.local', 'example.com', '127.0.0.1'], [
122+
// from cli url
123+
'https://example.com/apps/files/js/example.js',
124+
// from base url
125+
'https://nextcloud.local/apps/files/js/example.js',
126+
// http variant from trusted domains
127+
'http://nextcloud.local/apps/files/js/example.js',
128+
'http://example.com/apps/files/js/example.js',
129+
// trusted domains
130+
'https://127.0.0.1/apps/files/js/example.js',
131+
'http://127.0.0.1/apps/files/js/example.js',
132+
]
133+
],
134+
'wildcard trusted domains' => [
135+
'/apps/files/js/example.js', false, '', '', ['nextcloud.local', '*.example.com'], [
136+
// from base url
137+
'https://nextcloud.local/apps/files/js/example.js',
138+
// http variant from trusted domains
139+
'http://nextcloud.local/apps/files/js/example.js',
140+
// trusted domains with wild card are skipped
141+
]
142+
],
143+
'missing leading slash' => [
144+
'apps/files/js/example.js', false, 'https://nextcloud.local', '', ['nextcloud.local'], [
145+
// from cli url
146+
'https://nextcloud.local/apps/files/js/example.js',
147+
// http variant from trusted domains
148+
'http://nextcloud.local/apps/files/js/example.js',
149+
]
150+
],
151+
'keep web-root' => [
152+
'/apps/files/js/example.js', false, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [
153+
// from cli url (note that the CLI url has NO web root)
154+
'https://example.com/apps/files/js/example.js',
155+
// from base url
156+
'https://nextcloud.local/nextcloud/apps/files/js/example.js',
157+
// http variant from trusted domains
158+
'http://nextcloud.local/nextcloud/apps/files/js/example.js',
159+
// trusted domains with web-root
160+
'https://example.com/nextcloud/apps/files/js/example.js',
161+
'http://example.com/nextcloud/apps/files/js/example.js',
162+
'https://192.168.100.1/nextcloud/apps/files/js/example.js',
163+
'http://192.168.100.1/nextcloud/apps/files/js/example.js',
164+
]
165+
],
166+
// example if the URL is generated by the URL generator
167+
'keep web-root and web root in url' => [
168+
'/nextcloud/apps/files/js/example.js', false, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [
169+
// from cli url (note that the CLI url has NO web root)
170+
'https://example.com/apps/files/js/example.js',
171+
// from base url
172+
'https://nextcloud.local/nextcloud/apps/files/js/example.js',
173+
// http variant from trusted domains
174+
'http://nextcloud.local/nextcloud/apps/files/js/example.js',
175+
// trusted domains with web-root
176+
'https://example.com/nextcloud/apps/files/js/example.js',
177+
'http://example.com/nextcloud/apps/files/js/example.js',
178+
'https://192.168.100.1/nextcloud/apps/files/js/example.js',
179+
'http://192.168.100.1/nextcloud/apps/files/js/example.js',
180+
]
181+
],
182+
'remove web-root' => [
183+
'/.well-known/caldav', true, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [
184+
// from cli url (note that the CLI url has NO web root)
185+
'https://example.com/.well-known/caldav',
186+
// from base url
187+
'https://nextcloud.local/.well-known/caldav',
188+
// http variant from trusted domains
189+
'http://nextcloud.local/.well-known/caldav',
190+
'http://example.com/.well-known/caldav',
191+
// trusted domains with web-root
192+
'https://192.168.100.1/.well-known/caldav',
193+
'http://192.168.100.1/.well-known/caldav',
194+
]
195+
],
196+
// example if the URL is generated by the URL generator
197+
'remove web-root and web root in url' => [
198+
'/nextcloud/.well-known/caldav', true, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [
199+
// from cli url (note that the CLI url has NO web root)
200+
'https://example.com/.well-known/caldav',
201+
// from base url
202+
'https://nextcloud.local/.well-known/caldav',
203+
// http variant from trusted domains
204+
'http://nextcloud.local/.well-known/caldav',
205+
'http://example.com/.well-known/caldav',
206+
// trusted domains with web-root
207+
'https://192.168.100.1/.well-known/caldav',
208+
'http://192.168.100.1/.well-known/caldav',
209+
]
210+
],
211+
];
212+
}
213+
214+
}

‎lib/base.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ class OC {
104104
*/
105105
private static string $SUBURI = '';
106106
/**
107-
* the Nextcloud root path for http requests (e.g. nextcloud/)
107+
* the Nextcloud root path for http requests (e.g. /nextcloud)
108108
*/
109109
public static string $WEBROOT = '';
110110
/**

0 commit comments

Comments
 (0)