upstream/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/server.git synced 2026-04-15 22:11:17 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 25

refactor(settings): CheckServerResponseTrait always expect absolute path

Browse source 
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
This commit is contained in:
Ferdinand Thiessen 2024-09-12 20:44:59 +02:00
parent 9e979d42b4
commit be1cd7a308
No known key found for this signature in database
GPG key ID: 45FAE7268762B400
5 changed files with 70 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

64
apps/settings/lib/SetupChecks/CheckServerResponseTrait.php
View file

@ -37,36 +37,38 @@ trait CheckServerResponseTrait {
* 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<string> 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 */
@ -76,20 +78,23 @@ trait CheckServerResponseTrait {
/* 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, '/');
}
@ -97,26 +102,28 @@ 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 (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
* @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<int, IResponse>
*/
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) {
@ -130,11 +137,10 @@ trait CheckServerResponseTrait {
* @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<int, IResponse>
*/
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 {

4
apps/settings/lib/SetupChecks/DataDirectoryProtected.php
View file

@ -40,8 +40,8 @@ class DataDirectoryProtected implements ISetupCheck {
}
public function run(): SetupResult {
$datadir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValue('datadirectory', ''));
$dataUrl = '/' . $datadir . '/.ncdata';
$dataDir = str_replace(\OC::$SERVERROOT . '/', '', $this->config->getSystemValueString('datadirectory', ''));
$dataUrl = $this->urlGenerator->linkTo('', $dataDir . '/.ncdata');
$noResponse = true;
foreach ($this->runRequest('GET', $dataUrl, [ 'httpErrors' => false ]) as $response) {

2
apps/settings/lib/SetupChecks/WellKnownUrls.php
View file

@ -53,7 +53,7 @@ class WellKnownUrls implements ISetupCheck {
$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

45
apps/settings/tests/SetupChecks/CheckServerResponseTraitTest.php
View file

@ -51,16 +51,27 @@ class CheckServerResponseTraitTest extends TestCase {
/**
* @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 @@ class CheckServerResponseTraitTest extends TestCase {
*/
public function testGetTestUrls(
string $url,
bool $removeWebRoot,
bool $isRootRequest,
string $cliUrl,
string $webRoot,
array $trustedDomains,
@ -93,10 +104,13 @@ class CheckServerResponseTraitTest extends TestCase {
->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<string, list{string, bool, string, string, list<string>, list<string>}>
*/
public static function dataGetTestUrls(): array {
return [
'same cli and base URL' => [
@ -193,21 +207,6 @@ class CheckServerResponseTraitTest extends TestCase {
'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',
]
],
];
}

20
apps/settings/tests/SetupChecks/DataDirectoryProtectedTest.php
View file

@ -20,17 +20,17 @@ use Psr\Log\LoggerInterface;
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())
@ -74,14 +74,14 @@ class DataDirectoryProtectedTest extends TestCase {
$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, true],
'success: forbidden access with redirect' => [[200], SetupResult::SUCCESS, false],
@ -102,7 +102,7 @@ class DataDirectoryProtectedTest extends TestCase {
$this->config
->expects($this->once())
->method('getSystemValue')
->method('getSystemValueString')
->willReturn('');
$result = $this->setupcheck->run();