Skip to content

Commit

Permalink
feat: Make CheckServerResponseTrait public and provide as `OCP\Setu…
Browse files Browse the repository at this point in the history
…pCheck\CheckServerResponseTrait`

This trait is used by other apps for creating setup checks,
so we should provide it instead apps using private API.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
  • Loading branch information
susnux authored and nickvergessen committed Sep 16, 2024
1 parent ff0cab5 commit 7fbd518
Show file tree
Hide file tree
Showing 15 changed files with 49 additions and 40 deletions.
2 changes: 1 addition & 1 deletion apps/dav/lib/SetupChecks/WebdavEndpoint.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

namespace OCA\DAV\SetupChecks;

use OCA\Settings\SetupChecks\CheckServerResponseTrait;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down
1 change: 0 additions & 1 deletion apps/settings/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@
'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => $baseDir . '/../lib/SetupChecks/AllowedAdminRanges.php',
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => $baseDir . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => $baseDir . '/../lib/SetupChecks/BruteForceThrottler.php',
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => $baseDir . '/../lib/SetupChecks/CheckServerResponseTrait.php',
'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => $baseDir . '/../lib/SetupChecks/CheckUserCertificates.php',
'OCA\\Settings\\SetupChecks\\CodeIntegrity' => $baseDir . '/../lib/SetupChecks/CodeIntegrity.php',
'OCA\\Settings\\SetupChecks\\CronErrors' => $baseDir . '/../lib/SetupChecks/CronErrors.php',
Expand Down
1 change: 0 additions & 1 deletion apps/settings/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ class ComposerStaticInitSettings
'OCA\\Settings\\SetupChecks\\AllowedAdminRanges' => __DIR__ . '/..' . '/../lib/SetupChecks/AllowedAdminRanges.php',
'OCA\\Settings\\SetupChecks\\AppDirsWithDifferentOwner' => __DIR__ . '/..' . '/../lib/SetupChecks/AppDirsWithDifferentOwner.php',
'OCA\\Settings\\SetupChecks\\BruteForceThrottler' => __DIR__ . '/..' . '/../lib/SetupChecks/BruteForceThrottler.php',
'OCA\\Settings\\SetupChecks\\CheckServerResponseTrait' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckServerResponseTrait.php',
'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckUserCertificates.php',
'OCA\\Settings\\SetupChecks\\CodeIntegrity' => __DIR__ . '/..' . '/../lib/SetupChecks/CodeIntegrity.php',
'OCA\\Settings\\SetupChecks\\CronErrors' => __DIR__ . '/..' . '/../lib/SetupChecks/CronErrors.php',
Expand Down
1 change: 1 addition & 0 deletions apps/settings/lib/SetupChecks/DataDirectoryProtected.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down
3 changes: 2 additions & 1 deletion apps/settings/lib/SetupChecks/JavaScriptModules.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -43,7 +44,7 @@ public function run(): SetupResult {
$testFile = $this->urlGenerator->linkTo('settings', 'js/esm-test.mjs');

$noResponse = true;
foreach ($this->runHEAD($testFile) as $response) {
foreach ($this->runRequest('HEAD', $testFile) as $response) {
$noResponse = false;
if (preg_match('/(text|application)\/javascript/i', $response->getHeader('Content-Type'))) {
return SetupResult::success();
Expand Down
3 changes: 2 additions & 1 deletion apps/settings/lib/SetupChecks/JavaScriptSourceMaps.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -42,7 +43,7 @@ public function getName(): string {
public function run(): SetupResult {
$testFile = $this->urlGenerator->linkTo('settings', 'js/map-test.js.map');

foreach ($this->runHEAD($testFile) as $response) {
foreach ($this->runRequest('HEAD', $testFile) as $response) {
return SetupResult::success();
}

Expand Down
1 change: 1 addition & 0 deletions apps/settings/lib/SetupChecks/OcxProviders.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down
5 changes: 3 additions & 2 deletions apps/settings/lib/SetupChecks/SecurityHeaders.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -71,8 +72,8 @@ public function run(): SetupResult {
}
}

$xssfields = array_map('trim', explode(';', $response->getHeader('X-XSS-Protection')));
if (!in_array('1', $xssfields) || !in_array('mode=block', $xssfields)) {
$xssFields = array_map('trim', explode(';', $response->getHeader('X-XSS-Protection')));
if (!in_array('1', $xssFields) || !in_array('mode=block', $xssFields)) {
$msg .= $this->l10n->t('- The `%1$s` HTTP header does not contain `%2$s`. This is a potential security or privacy risk, as it is recommended to adjust this setting accordingly.', ['X-XSS-Protection', '1; mode=block'])."\n";
}

Expand Down
1 change: 1 addition & 0 deletions apps/settings/lib/SetupChecks/WellKnownUrls.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down
3 changes: 2 additions & 1 deletion apps/settings/lib/SetupChecks/Woff2Loading.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use OCP\SetupCheck\ISetupCheck;
use OCP\SetupCheck\SetupResult;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -49,7 +50,7 @@ public function run(): SetupResult {

protected function checkFont(string $fileExtension, string $url): SetupResult {
$noResponse = true;
$responses = $this->runHEAD($url);
$responses = $this->runRequest('HEAD', $url);
foreach ($responses as $response) {
$noResponse = false;
if ($response->getStatusCode() === 200) {
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,7 @@
'OCP\\Settings\\IManager' => $baseDir . '/lib/public/Settings/IManager.php',
'OCP\\Settings\\ISettings' => $baseDir . '/lib/public/Settings/ISettings.php',
'OCP\\Settings\\ISubAdminSettings' => $baseDir . '/lib/public/Settings/ISubAdminSettings.php',
'OCP\\SetupCheck\\CheckServerResponseTrait' => $baseDir . '/lib/public/SetupCheck/CheckServerResponseTrait.php',
'OCP\\SetupCheck\\ISetupCheck' => $baseDir . '/lib/public/SetupCheck/ISetupCheck.php',
'OCP\\SetupCheck\\ISetupCheckManager' => $baseDir . '/lib/public/SetupCheck/ISetupCheckManager.php',
'OCP\\SetupCheck\\SetupResult' => $baseDir . '/lib/public/SetupCheck/SetupResult.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Settings\\IManager' => __DIR__ . '/../../..' . '/lib/public/Settings/IManager.php',
'OCP\\Settings\\ISettings' => __DIR__ . '/../../..' . '/lib/public/Settings/ISettings.php',
'OCP\\Settings\\ISubAdminSettings' => __DIR__ . '/../../..' . '/lib/public/Settings/ISubAdminSettings.php',
'OCP\\SetupCheck\\CheckServerResponseTrait' => __DIR__ . '/../../..' . '/lib/public/SetupCheck/CheckServerResponseTrait.php',
'OCP\\SetupCheck\\ISetupCheck' => __DIR__ . '/../../..' . '/lib/public/SetupCheck/ISetupCheck.php',
'OCP\\SetupCheck\\ISetupCheckManager' => __DIR__ . '/../../..' . '/lib/public/SetupCheck/ISetupCheckManager.php',
'OCP\\SetupCheck\\SetupResult' => __DIR__ . '/../../..' . '/lib/public/SetupCheck/SetupResult.php',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,33 @@
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Settings\SetupChecks;
namespace OCP\SetupCheck;

use Generator;
use OCP\Http\Client\IClientService;
use OCP\Http\Client\IResponse;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\L10N\IFactory;
use Psr\Log\LoggerInterface;

/**
* Common trait for setup checks that need to use requests to the same server and check the response
* @since 31.0.0
*/
trait CheckServerResponseTrait {
protected IConfig $config;
protected IURLGenerator $urlGenerator;
protected IClientService $clientService;
protected IL10N $l10n;
protected LoggerInterface $logger;

/**
* Common helper string in case a check could not fetch any results
* @since 31.0.0
*/
protected function serverConfigHelp(): string {
return $this->l10n->t('To allow this check to run you have to make sure that your Web server can connect to itself. Therefore it must be able to resolve and connect to at least one of its `trusted_domains` or the `overwrite.cli.url`. This failure may be the result of a server-side DNS mismatch or outbound firewall rule.');
$l10n = \OCP\Server::get(IFactory::class)->get('lib');
return $l10n->t('To allow this check to run you have to make sure that your Web server can connect to itself. Therefore it must be able to resolve and connect to at least one of its `trusted_domains` or the `overwrite.cli.url`. This failure may be the result of a server-side DNS mismatch or outbound firewall rule.');
}

/**
Expand All @@ -40,6 +42,7 @@ protected function serverConfigHelp(): string {
* @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
* @since 31.0.0
*/
protected function getTestUrls(string $url, bool $isRootRequest = false): array {
$url = '/' . ltrim($url, '/');
Expand Down Expand Up @@ -85,20 +88,6 @@ protected function getTestUrls(string $url, bool $isRootRequest = false): array
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, bool $removeWebroot): string {
if ($removeWebroot) {
$segments = parse_url($url);
$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;
}
return rtrim($url, '/');
}

/**
* Run a HTTP request to check header
* @param string $method The HTTP method to use
Expand All @@ -115,6 +104,7 @@ protected function normalizeUrl(string $url, bool $removeWebroot): string {
* ]
*
* @return Generator<int, IResponse>
* @since 31.0.0
*/
protected function runRequest(string $method, string $url, array $options = [], bool $isRootRequest = false): Generator {
$options = array_merge(['ignoreSSL' => true, 'httpErrors' => true], $options);
Expand All @@ -133,17 +123,11 @@ protected function runRequest(string $method, string $url, array $options = [],
}

/**
* Run a HEAD request to check header
* @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)
* @return Generator<int, IResponse>
* Get HTTP client options
* @param bool $ignoreSSL If set SSL errors are ignored (e.g. self-signed certificates)
* @since 31.0.0
*/
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 {
private function getRequestOptions(bool $ignoreSSL, bool $httpErrors): array {
$requestOptions = [
'connect_timeout' => 10,
'http_errors' => $httpErrors,
Expand All @@ -156,4 +140,23 @@ protected function getRequestOptions(bool $ignoreSSL, bool $httpErrors): array {
}
return $requestOptions;
}

/**
* 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
* @since 31.0.0
*/
private function normalizeUrl(string $url, bool $removeWebroot): string {
if ($removeWebroot) {
$segments = parse_url($url);
if (!isset($segments['scheme']) || !isset($segments['host'])) {
throw new \InvalidArgumentException('URL is missing scheme or host');
}

$port = isset($segments['port']) ? (':' . $segments['port']) : '';
return $segments['scheme'] . '://' . $segments['host'] . $port;
}
return rtrim($url, '/');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Settings\Tests\SetupChecks;
namespace Test\SetupCheck;

use OCA\Settings\SetupChecks\CheckServerResponseTrait;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IL10N;
use OCP\IURLGenerator;
use OCP\SetupCheck\CheckServerResponseTrait;
use Psr\Log\LoggerInterface;

/**
Expand All @@ -22,7 +22,6 @@ class CheckServerResponseTraitImplementation {

use CheckServerResponseTrait {
CheckServerResponseTrait::getRequestOptions as public;
CheckServerResponseTrait::runHEAD as public;
CheckServerResponseTrait::runRequest as public;
CheckServerResponseTrait::normalizeUrl as public;
CheckServerResponseTrait::getTestUrls as public;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/
namespace OCA\Settings\Tests\SetupChecks;
namespace Test\SetupCheck;

use OCP\Http\Client\IClientService;
use OCP\IConfig;
Expand Down

0 comments on commit 7fbd518

Please sign in to comment.