diff --git a/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php b/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php index 8bb1c33a12fc5..160fb7b3a8cec 100644 --- a/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php +++ b/apps/settings/lib/SetupChecks/CheckServerResponseTrait.php @@ -54,36 +54,38 @@ protected function serverConfigHelp(): string { * Get all possible URLs that need to be checked for a local request test. * This takes all `trusted_domains` and the CLI overwrite URL into account. * - * @param string $url The relative URL to test starting with a / + * @param string $url The absolute path (absolute URL without host but with web-root) to test starting with a / + * @param bool $isRootRequest Set to remove the web-root from URL and host (e.g. when requesting a path in the domain root like '/.well-known') * @return list List of possible absolute URLs */ - protected function getTestUrls(string $url, bool $removeWebroot): array { + protected function getTestUrls(string $url, bool $isRootRequest = false): array { $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)) { + if ($isRootRequest === false && $webroot !== '' && str_starts_with($url, $webroot)) { + // The URL contains the web-root but also the base url does so, + // so we need to remove the web-root from the URL. $url = substr($url, strlen($webroot)); } - $hosts = []; + // Base URLs to test + $baseUrls = []; - /* Try overwrite.cli.url first, it’s supposed to be how the server contacts itself */ + // Try overwrite.cli.url first, it’s supposed to be how the server contacts itself $cliUrl = $this->config->getSystemValueString('overwrite.cli.url', ''); if ($cliUrl !== '') { - $hosts[] = $this->normalizeUrl( + // The CLI URL already contains the web-root, so we need to normalize it if requested + $baseUrls[] = $this->normalizeUrl( $cliUrl, - $webroot, - $removeWebroot + $isRootRequest ); } - /* Try URL generator second */ - $hosts[] = $this->normalizeUrl( + // Try URL generator second + // The base URL also contains the webroot so also normalize it + $baseUrls[] = $this->normalizeUrl( $this->urlGenerator->getBaseUrl(), - $webroot, - $removeWebroot + $isRootRequest ); /* Last resort: trusted domains */ @@ -93,20 +95,23 @@ protected function getTestUrls(string $url, bool $removeWebroot): array { /* Ignore domains with a wildcard */ continue; } - $hosts[] = $this->normalizeUrl("https://$host$webroot", $webroot, $removeWebroot); - $hosts[] = $this->normalizeUrl("http://$host$webroot", $webroot, $removeWebroot); + $baseUrls[] = $this->normalizeUrl("https://$host$webroot", $isRootRequest); + $baseUrls[] = $this->normalizeUrl("http://$host$webroot", $isRootRequest); } - return array_map(fn (string $host) => $host . $url, array_values(array_unique($hosts))); + return array_map(fn (string $host) => $host . $url, array_values(array_unique($baseUrls))); } /** * Strip a trailing slash and remove the webroot if requested. + * @param string $url The URL to normalize. Should be an absolute URL containing scheme, host and optionally web-root. + * @param bool $removeWebroot If set the web-root is removed from the URL and an absolute URL with only the scheme and host (optional port) is returned */ - protected function normalizeUrl(string $url, string $webroot, bool $removeWebroot): string { - $url = rtrim($url, '/'); - if ($removeWebroot && $webroot !== '' && str_ends_with($url, $webroot)) { - $url = substr($url, 0, -strlen($webroot)); + protected function normalizeUrl(string $url, bool $removeWebroot): string { + if ($removeWebroot) { + $segments = parse_url($url); + $port = isset($segments['port']) ? (':' . $segments['port']) : ''; + return $segments['scheme'] . '://' . $segments['host'] . $port; } return rtrim($url, '/'); } @@ -114,26 +119,28 @@ protected function normalizeUrl(string $url, string $webroot, bool $removeWebroo /** * Run a HTTP request to check header * @param string $method The HTTP method to use - * @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) - * 'ignoreSSL' => true, - * // Ignore requests with HTTP errors (will not yield if request has a 4xx or 5xx response) - * 'httpErrors' => true, - * ] + * @param string $url The absolute path (URL with webroot but without host) to check, can be the output of `IURLGenerator` + * @param bool $isRootRequest If set the webroot is removed from URLs to make the request target the host's root. Example usage are the /.well-known URLs in the root path. + * @param array{ignoreSSL?: bool, httpErrors?: bool, options?: array} $options HTTP client related options, like + * [ + * // Ignore invalid SSL certificates (e.g. self signed) + * 'ignoreSSL' => true, + * // Ignore requests with HTTP errors (will not yield if request has a 4xx or 5xx response) + * 'httpErrors' => true, + * // Additional options for the HTTP client (see `IClient`) + * 'options' => [], + * ] * * @return Generator */ - protected function runRequest(string $method, string $url, array $options = [], bool $removeWebroot = false): Generator { + protected function runRequest(string $method, string $url, array $options = [], bool $isRootRequest = false): Generator { $options = array_merge(['ignoreSSL' => true, 'httpErrors' => true], $options); $client = $this->clientService->newClient(); $requestOptions = $this->getRequestOptions($options['ignoreSSL'], $options['httpErrors']); $requestOptions = array_merge($requestOptions, $options['options'] ?? []); - foreach ($this->getTestUrls($url, $removeWebroot) as $testURL) { + foreach ($this->getTestUrls($url, $isRootRequest) as $testURL) { try { yield $client->request($method, $testURL, $requestOptions); } catch (\Throwable $e) { @@ -147,11 +154,10 @@ protected function runRequest(string $method, string $url, array $options = [], * @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, bool $removeWebroot = false): Generator { - return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors], $removeWebroot); + protected function runHEAD(string $url, bool $ignoreSSL = true, bool $httpErrors = true): Generator { + return $this->runRequest('HEAD', $url, ['ignoreSSL' => $ignoreSSL, 'httpErrors' => $httpErrors]); } 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 6bfabc6057c93..96f054ade5ea5 100644 --- a/apps/settings/lib/SetupChecks/DataDirectoryProtected.php +++ b/apps/settings/lib/SetupChecks/DataDirectoryProtected.php @@ -57,8 +57,8 @@ public function getName(): string { } public function run(): SetupResult { - $datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', '')); - $dataUrl = '/' . $datadir . '/.ocdata'; + $dataDir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValueString('datadirectory', '')); + $dataUrl = $this->urlGenerator->linkTo('', $dataDir . '/.ocdata'); $noResponse = true; foreach ($this->runHEAD($dataUrl, httpErrors:false) as $response) { diff --git a/apps/settings/lib/SetupChecks/WellKnownUrls.php b/apps/settings/lib/SetupChecks/WellKnownUrls.php index 4d69826d2488d..39540b400e36f 100644 --- a/apps/settings/lib/SetupChecks/WellKnownUrls.php +++ b/apps/settings/lib/SetupChecks/WellKnownUrls.php @@ -71,7 +71,7 @@ public function run(): SetupResult { $requestOptions = ['httpErrors' => false, 'options' => ['allow_redirects' => ['track_redirects' => true]]]; foreach ($urls as [$verb,$url,$validStatuses,$checkCustomHeader]) { $works = null; - foreach ($this->runRequest($verb, $url, $requestOptions, removeWebroot: true) as $response) { + foreach ($this->runRequest($verb, $url, $requestOptions, isRootRequest: 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/CheckServerResponseTraitTest.php b/apps/settings/tests/SetupChecks/CheckServerResponseTraitTest.php index 5318955e42145..e51546c1cf174 100644 --- a/apps/settings/tests/SetupChecks/CheckServerResponseTraitTest.php +++ b/apps/settings/tests/SetupChecks/CheckServerResponseTraitTest.php @@ -51,16 +51,27 @@ protected function setUp(): void { /** * @dataProvider dataNormalizeUrl */ - public function testNormalizeUrl(string $url, string $webRoot, bool $removeWebRoot, string $expected): void { - $this->assertEquals($expected, $this->trait->normalizeUrl($url, $webRoot, $removeWebRoot)); + public function testNormalizeUrl(string $url, bool $isRootRequest, string $expected): void { + $this->assertEquals($expected, $this->trait->normalizeUrl($url, $isRootRequest)); } 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'], + // untouched web-root + 'valid and nothing to change' => ['http://example.com/root', false, 'http://example.com/root'], + 'valid with port and nothing to change' => ['http://example.com:8081/root', false, 'http://example.com:8081/root'], + 'trailing slash' => ['http://example.com/root/', false, 'http://example.com/root'], + 'deep web root' => ['http://example.com/deep/webroot', false, 'http://example.com/deep/webroot'], + // removal of the web-root + 'remove web root' => ['http://example.com/root/', true, 'http://example.com'], + 'remove web root but empty' => ['http://example.com', true, 'http://example.com'], + 'remove deep web root' => ['http://example.com/deep/webroot', true, 'http://example.com'], + 'remove web root with port' => ['http://example.com:8081/root', true, 'http://example.com:8081'], + 'remove web root with port but empty' => ['http://example.com:8081', true, 'http://example.com:8081'], + 'remove web root from IP' => ['https://127.0.0.1/root', true, 'https://127.0.0.1'], + 'remove web root from IP with port' => ['https://127.0.0.1:8080/root', true, 'https://127.0.0.1:8080'], + 'remove web root from IPv6' => ['https://[ff02::1]/root', true, 'https://[ff02::1]'], + 'remove web root from IPv6 with port' => ['https://[ff02::1]:8080/root', true, 'https://[ff02::1]:8080'], ]; } @@ -69,7 +80,7 @@ public static function dataNormalizeUrl(): array { */ public function testGetTestUrls( string $url, - bool $removeWebRoot, + bool $isRootRequest, string $cliUrl, string $webRoot, array $trustedDomains, @@ -93,10 +104,13 @@ public function testGetTestUrls( ->method('getBaseUrl') ->willReturn(self::BASE_URL . $webRoot); - $result = $this->trait->getTestUrls($url, $removeWebRoot); + $result = $this->trait->getTestUrls($url, $isRootRequest); $this->assertEquals($expected, $result); } + /** + * @return array, list}> + */ public static function dataGetTestUrls(): array { return [ 'same cli and base URL' => [ @@ -193,21 +207,6 @@ public static function dataGetTestUrls(): array { '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/apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php b/apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php index 8eec24a51c4bb..1e7d034f39eaa 100644 --- a/apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php +++ b/apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php @@ -37,17 +37,17 @@ use Test\TestCase; class DataDirectoryProtectedTest extends TestCase { - private IL10N|MockObject $l10n; - private IConfig|MockObject $config; - private IURLGenerator|MockObject $urlGenerator; - private IClientService|MockObject $clientService; - private LoggerInterface|MockObject $logger; - private DataDirectoryProtected|MockObject $setupcheck; + private IL10N&MockObject $l10n; + private IConfig&MockObject $config; + private IURLGenerator&MockObject $urlGenerator; + private IClientService&MockObject $clientService; + private LoggerInterface&MockObject $logger; + private DataDirectoryProtected&MockObject $setupcheck; protected function setUp(): void { parent::setUp(); - /** @var IL10N|MockObject */ + /** @var IL10N&MockObject */ $this->l10n = $this->getMockBuilder(IL10N::class) ->disableOriginalConstructor()->getMock(); $this->l10n->expects($this->any()) @@ -90,14 +90,14 @@ public function testStatusCode(array $status, string $expected): void { $this->config ->expects($this->once()) - ->method('getSystemValue') + ->method('getSystemValueString') ->willReturn(''); $result = $this->setupcheck->run(); $this->assertEquals($expected, $result->getSeverity()); } - public function dataTestStatusCode(): array { + public static function dataTestStatusCode(): array { return [ 'success: forbidden access' => [[403], SetupResult::SUCCESS], 'error: can access' => [[200], SetupResult::ERROR], @@ -117,7 +117,7 @@ public function testNoResponse(): void { $this->config ->expects($this->once()) - ->method('getSystemValue') + ->method('getSystemValueString') ->willReturn(''); $result = $this->setupcheck->run();