diff --git a/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php b/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php index d4e0567cfaf..d3a5af58782 100644 --- a/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php +++ b/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php @@ -38,49 +38,49 @@ trait CheckServerResponseTrait { * This takes all `trusted_domains` and the CLI overwrite URL into account. * * @param string $url The relative URL to test starting with a / - * @return string[] List of possible absolute URLs + * @return list List of possible absolute URLs */ protected function getTestUrls(string $url, bool $removeWebroot): array { - $testUrls = []; + $url = '/' . ltrim($url, '/'); $webroot = rtrim($this->urlGenerator->getWebroot(), '/'); + // Similar to `getAbsoluteURL` of URLGenerator: + // The Nextcloud web root could already be prepended. + if ($webroot !== '' && str_starts_with($url, $webroot)) { + $url = substr($url, strlen($webroot)); + } + + $hosts = []; /* Try overwrite.cli.url first, it’s supposed to be how the server contacts itself */ $cliUrl = $this->config->getSystemValueString('overwrite.cli.url', ''); - if ($cliUrl !== '') { - $cliUrl = $this->normalizeUrl( + $hosts[] = $this->normalizeUrl( $cliUrl, $webroot, $removeWebroot ); - - $testUrls[] = $cliUrl . $url; } /* Try URL generator second */ - $baseUrl = $this->normalizeUrl( + $hosts[] = $this->normalizeUrl( $this->urlGenerator->getBaseUrl(), $webroot, $removeWebroot ); - if ($baseUrl !== $cliUrl) { - $testUrls[] = $baseUrl . $url; - } - /* Last resort: trusted domains */ - $hosts = $this->config->getSystemValue('trusted_domains', []); - foreach ($hosts as $host) { + $trustedDomains = $this->config->getSystemValue('trusted_domains', []); + foreach ($trustedDomains as $host) { if (str_contains($host, '*')) { /* Ignore domains with a wildcard */ continue; } - $hosts[] = 'https://' . $host . $url; - $hosts[] = 'http://' . $host . $url; + $hosts[] = $this->normalizeUrl("https://$host$webroot", $webroot, $removeWebroot); + $hosts[] = $this->normalizeUrl("http://$host$webroot", $webroot, $removeWebroot); } - return $testUrls; + return array_map(fn (string $host) => $host . $url, array_values(array_unique($hosts))); } /** @@ -88,8 +88,8 @@ trait CheckServerResponseTrait { */ protected function normalizeUrl(string $url, string $webroot, bool $removeWebroot): string { $url = rtrim($url, '/'); - if ($removeWebroot && str_ends_with($url, $webroot)) { - $url = substr($url, -strlen($webroot)); + if ($removeWebroot && $webroot !== '' && str_ends_with($url, $webroot)) { + $url = substr($url, 0, -strlen($webroot)); } return rtrim($url, '/'); } @@ -97,7 +97,8 @@ trait CheckServerResponseTrait { /** * Run a HTTP request to check header * @param string $method The HTTP method to use - * @param string $url The relative URL to check + * @param string $url The relative URL to check (e.g. output of IURLGenerator) + * @param bool $removeWebroot Remove the webroot from the URL (handle URL as relative to domain root) * @param array{ignoreSSL?: bool, httpErrors?: bool, options?: array} $options Additional options, like * [ * // Ignore invalid SSL certificates (e.g. self signed) @@ -126,13 +127,14 @@ trait CheckServerResponseTrait { /** * Run a HEAD request to check header - * @param string $url The relative URL to check + * @param string $url The relative URL to check (e.g. output of IURLGenerator) * @param bool $ignoreSSL Ignore SSL certificates * @param bool $httpErrors Ignore requests with HTTP errors (will not yield if request has a 4xx or 5xx response) + * @param bool $removeWebroot Remove the webroot from the URL (handle URL as relative to domain root) * @return Generator */ - protected function runHEAD(string $url, bool $ignoreSSL = true, bool $httpErrors = true): Generator { - return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors]); + protected function runHEAD(string $url, bool $ignoreSSL = true, bool $httpErrors = true, bool $removeWebroot = false): Generator { + return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors], $removeWebroot); } protected function getRequestOptions(bool $ignoreSSL, bool $httpErrors): array { diff --git a/apps/settings/lib/SetupChecks/DataDirectoryProtected.php b/apps/settings/lib/SetupChecks/DataDirectoryProtected.php index 447c66d5c5d..ff7d3b5a3a9 100644 --- a/apps/settings/lib/SetupChecks/DataDirectoryProtected.php +++ b/apps/settings/lib/SetupChecks/DataDirectoryProtected.php @@ -41,8 +41,7 @@ class DataDirectoryProtected implements ISetupCheck { public function run(): SetupResult { $datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', '')); - - $dataUrl = $this->urlGenerator->getWebroot() . '/' . $datadir . '/.ncdata'; + $dataUrl = '/' . $datadir . '/.ncdata'; $noResponse = true; foreach ($this->runRequest('GET', $dataUrl, [ 'httpErrors' => false ]) as $response) { diff --git a/apps/settings/lib/SetupChecks/WellKnownUrls.php b/apps/settings/lib/SetupChecks/WellKnownUrls.php index 565544cfdd7..03779b9f28a 100644 --- a/apps/settings/lib/SetupChecks/WellKnownUrls.php +++ b/apps/settings/lib/SetupChecks/WellKnownUrls.php @@ -50,9 +50,10 @@ class WellKnownUrls implements ISetupCheck { ['propfind', '/.well-known/carddav', [207], false], ]; + $requestOptions = ['httpErrors' => false, 'options' => ['allow_redirects' => ['track_redirects' => true]]]; foreach ($urls as [$verb,$url,$validStatuses,$checkCustomHeader]) { $works = null; - foreach ($this->runRequest($verb, $url, ['httpErrors' => false, 'options' => ['allow_redirects' => ['track_redirects' => true]]], removeWebroot: true) as $response) { + foreach ($this->runRequest($verb, $url, $requestOptions, removeWebroot: true) as $response) { // Check that the response status matches $works = in_array($response->getStatusCode(), $validStatuses); // and (if needed) the custom Nextcloud header is set diff --git a/apps/settings/tests/SetupChecks/CheckServerResponseTraitImplementation.php b/apps/settings/tests/SetupChecks/CheckServerResponseTraitImplementation.php new file mode 100644 index 00000000000..6c8b65855cc --- /dev/null +++ b/apps/settings/tests/SetupChecks/CheckServerResponseTraitImplementation.php @@ -0,0 +1,40 @@ +l10n = $this->createMock(IL10N::class); + $this->l10n->method('t') + ->willReturnArgument(0); + $this->config = $this->createMock(IConfig::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->clientService = $this->createMock(IClientService::class); + $this->logger = $this->createMock(LoggerInterface::class); + + $this->trait = new CheckServerResponseTraitImplementation( + $this->l10n, + $this->config, + $this->urlGenerator, + $this->clientService, + $this->logger, + ); + } + + /** + * @dataProvider dataNormalizeUrl + */ + public function testNormalizeUrl(string $url, string $webRoot, bool $removeWebRoot, string $expected): void { + $this->assertEquals($expected, $this->trait->normalizeUrl($url, $webRoot, $removeWebRoot)); + } + + public static function dataNormalizeUrl(): array { + return [ + 'valid and nothing to change' => ['http://example.com/root', '/root', false, 'http://example.com/root'], + 'trailing slash' => ['http://example.com/root/', '/root', false, 'http://example.com/root'], + 'remove web root' => ['http://example.com/root/', '/root', true, 'http://example.com'], + 'remove web root but empty' => ['http://example.com', '', true, 'http://example.com'], + ]; + } + + /** + * @dataProvider dataGetTestUrls + */ + public function testGetTestUrls( + string $url, + bool $removeWebRoot, + string $cliUrl, + string $webRoot, + array $trustedDomains, + array $expected, + ): void { + $this->config->expects(self::atLeastOnce()) + ->method('getSystemValueString') + ->with('overwrite.cli.url', '') + ->willReturn($cliUrl); + + $this->config->expects(self::atLeastOnce()) + ->method('getSystemValue') + ->with('trusted_domains', []) + ->willReturn($trustedDomains); + + $this->urlGenerator->expects(self::atLeastOnce()) + ->method('getWebroot') + ->willReturn($webRoot); + + $this->urlGenerator->expects(self::atLeastOnce()) + ->method('getBaseUrl') + ->willReturn(self::BASE_URL . $webRoot); + + $result = $this->trait->getTestUrls($url, $removeWebRoot); + $this->assertEquals($expected, $result); + } + + public static function dataGetTestUrls(): array { + return [ + 'same cli and base URL' => [ + '/apps/files/js/example.js', false, 'https://nextcloud.local', '', ['nextcloud.local'], [ + // from cli url + 'https://nextcloud.local/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/apps/files/js/example.js', + ] + ], + 'different cli and base URL' => [ + '/apps/files/js/example.js', false, 'https://example.com', '', ['nextcloud.local'], [ + // from cli url + 'https://example.com/apps/files/js/example.js', + // from base url + 'https://nextcloud.local/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/apps/files/js/example.js', + ] + ], + 'different cli and base URL and trusted domains' => [ + '/apps/files/js/example.js', false, 'https://example.com', '', ['nextcloud.local', 'example.com', '127.0.0.1'], [ + // from cli url + 'https://example.com/apps/files/js/example.js', + // from base url + 'https://nextcloud.local/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/apps/files/js/example.js', + 'http://example.com/apps/files/js/example.js', + // trusted domains + 'https://127.0.0.1/apps/files/js/example.js', + 'http://127.0.0.1/apps/files/js/example.js', + ] + ], + 'wildcard trusted domains' => [ + '/apps/files/js/example.js', false, '', '', ['nextcloud.local', '*.example.com'], [ + // from base url + 'https://nextcloud.local/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/apps/files/js/example.js', + // trusted domains with wild card are skipped + ] + ], + 'missing leading slash' => [ + 'apps/files/js/example.js', false, 'https://nextcloud.local', '', ['nextcloud.local'], [ + // from cli url + 'https://nextcloud.local/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/apps/files/js/example.js', + ] + ], + 'keep web-root' => [ + '/apps/files/js/example.js', false, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [ + // from cli url (note that the CLI url has NO web root) + 'https://example.com/apps/files/js/example.js', + // from base url + 'https://nextcloud.local/nextcloud/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/nextcloud/apps/files/js/example.js', + // trusted domains with web-root + 'https://example.com/nextcloud/apps/files/js/example.js', + 'http://example.com/nextcloud/apps/files/js/example.js', + 'https://192.168.100.1/nextcloud/apps/files/js/example.js', + 'http://192.168.100.1/nextcloud/apps/files/js/example.js', + ] + ], + // example if the URL is generated by the URL generator + 'keep web-root and web root in url' => [ + '/nextcloud/apps/files/js/example.js', false, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [ + // from cli url (note that the CLI url has NO web root) + 'https://example.com/apps/files/js/example.js', + // from base url + 'https://nextcloud.local/nextcloud/apps/files/js/example.js', + // http variant from trusted domains + 'http://nextcloud.local/nextcloud/apps/files/js/example.js', + // trusted domains with web-root + 'https://example.com/nextcloud/apps/files/js/example.js', + 'http://example.com/nextcloud/apps/files/js/example.js', + 'https://192.168.100.1/nextcloud/apps/files/js/example.js', + 'http://192.168.100.1/nextcloud/apps/files/js/example.js', + ] + ], + 'remove web-root' => [ + '/.well-known/caldav', true, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [ + // from cli url (note that the CLI url has NO web root) + 'https://example.com/.well-known/caldav', + // from base url + 'https://nextcloud.local/.well-known/caldav', + // http variant from trusted domains + 'http://nextcloud.local/.well-known/caldav', + 'http://example.com/.well-known/caldav', + // trusted domains with web-root + 'https://192.168.100.1/.well-known/caldav', + 'http://192.168.100.1/.well-known/caldav', + ] + ], + // example if the URL is generated by the URL generator + 'remove web-root and web root in url' => [ + '/nextcloud/.well-known/caldav', true, 'https://example.com', '/nextcloud', ['nextcloud.local', 'example.com', '192.168.100.1'], [ + // from cli url (note that the CLI url has NO web root) + 'https://example.com/.well-known/caldav', + // from base url + 'https://nextcloud.local/.well-known/caldav', + // http variant from trusted domains + 'http://nextcloud.local/.well-known/caldav', + 'http://example.com/.well-known/caldav', + // trusted domains with web-root + 'https://192.168.100.1/.well-known/caldav', + 'http://192.168.100.1/.well-known/caldav', + ] + ], + ]; + } + +} diff --git a/lib/base.php b/lib/base.php index 4e66d389e7b..7654aef2b60 100644 --- a/lib/base.php +++ b/lib/base.php @@ -43,7 +43,7 @@ class OC { */ private static string $SUBURI = ''; /** - * the Nextcloud root path for http requests (e.g. nextcloud/) + * the Nextcloud root path for http requests (e.g. /nextcloud) */ public static string $WEBROOT = ''; /** diff --git a/lib/private/URLGenerator.php b/lib/private/URLGenerator.php index ab568a3296d..39b825e427b 100644 --- a/lib/private/URLGenerator.php +++ b/lib/private/URLGenerator.php @@ -245,7 +245,7 @@ class URLGenerator implements IURLGenerator { /** * Makes an URL absolute - * @param string $url the url in the ownCloud host + * @param string $url the url in the Nextcloud host * @return string the absolute version of the url */ public function getAbsoluteURL(string $url): string { @@ -254,7 +254,7 @@ class URLGenerator implements IURLGenerator { if (\OC::$CLI && !\defined('PHPUNIT_RUN')) { return rtrim($this->config->getSystemValueString('overwrite.cli.url'), '/') . '/' . ltrim($url, '/'); } - // The ownCloud web root can already be prepended. + // The Nextcloud web root could already be prepended. if (\OC::$WEBROOT !== '' && str_starts_with($url, \OC::$WEBROOT)) { $url = substr($url, \strlen(\OC::$WEBROOT)); }