Merge pull request #21693 from nextcloud/fix/noid/import-certificates-only-by-system

Improve CertificateManager to not be user context dependent

9 files changed, 98 insertions, 19 deletions

diff --git a/apps/dav/lib/CardDAV/SyncService.php b/apps/dav/lib/CardDAV/SyncService.php
index 1cd01066a7c..bcb20409524 100644
--- a/apps/dav/lib/CardDAV/SyncService.php
+++ b/apps/dav/lib/CardDAV/SyncService.php
@@ -31,7 +31,6 @@ namespace OCA\DAV\CardDAV;
 
 use OC\Accounts\AccountManager;
 use OCP\AppFramework\Http;
-use OCP\ICertificateManager;
 use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
@@ -155,8 +154,7 @@ class SyncService {
 			return $this->certPath;
 		}
 
-		/** @var ICertificateManager $certManager */
-		$certManager = \OC::$server->getCertificateManager(null);
+		$certManager = \OC::$server->getCertificateManager();
 		$certPath = $certManager->getAbsoluteBundlePath();
 		if (file_exists($certPath)) {
 			$this->certPath = $certPath;
diff --git a/apps/files_sharing/lib/External/Manager.php b/apps/files_sharing/lib/External/Manager.php
index c922754207c..87146dd4268 100644
--- a/apps/files_sharing/lib/External/Manager.php
+++ b/apps/files_sharing/lib/External/Manager.php
@@ -441,7 +441,7 @@ class Manager {
 		$data['manager'] = $this;
 		$mountPoint = '/' . $this->uid . '/files' . $data['mountpoint'];
 		$data['mountpoint'] = $mountPoint;
-		$data['certificateManager'] = \OC::$server->getCertificateManager($this->uid);
+		$data['certificateManager'] = \OC::$server->getCertificateManager();
 		return new Mount(self::STORAGE, $mountPoint, $data, $this, $this->storageLoader);
 	}
 
diff --git a/apps/files_sharing/lib/External/MountProvider.php b/apps/files_sharing/lib/External/MountProvider.php
index ecff358abe5..eb8f1b8fde6 100644
--- a/apps/files_sharing/lib/External/MountProvider.php
+++ b/apps/files_sharing/lib/External/MountProvider.php
@@ -66,7 +66,7 @@ class MountProvider implements IMountProvider {
 		$mountPoint = '/' . $user->getUID() . '/files/' . ltrim($data['mountpoint'], '/');
 		$data['mountpoint'] = $mountPoint;
 		$data['cloudId'] = $this->cloudIdManager->getCloudId($data['owner'], $data['remote']);
-		$data['certificateManager'] = \OC::$server->getCertificateManager($user->getUID());
+		$data['certificateManager'] = \OC::$server->getCertificateManager();
 		$data['HttpClientService'] = \OC::$server->getHTTPClientService();
 		return new Mount(self::STORAGE, $mountPoint, $data, $manager, $storageFactory);
 	}
diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php
index 3ccd7d9d030..fbe1d1aa4d0 100644
--- a/apps/settings/composer/composer/autoload_classmap.php
+++ b/apps/settings/composer/composer/autoload_classmap.php
@@ -56,6 +56,7 @@ return array(
     'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php',
     'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => $baseDir . '/../lib/Settings/Personal/Security/WebAuthn.php',
     'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/../lib/Settings/Personal/ServerDevNotice.php',
+    'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => $baseDir . '/../lib/SetupChecks/CheckUserCertificates.php',
     'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => $baseDir . '/../lib/SetupChecks/LegacySSEKeyFormat.php',
     'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => $baseDir . '/../lib/SetupChecks/PhpDefaultCharset.php',
     'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => $baseDir . '/../lib/SetupChecks/PhpOutputBuffering.php',
diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php
index bf831a81cd4..dda9fa778cc 100644
--- a/apps/settings/composer/composer/autoload_static.php
+++ b/apps/settings/composer/composer/autoload_static.php
@@ -71,6 +71,7 @@ class ComposerStaticInitSettings
         'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php',
         'OCA\\Settings\\Settings\\Personal\\Security\\WebAuthn' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/WebAuthn.php',
         'OCA\\Settings\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/..' . '/../lib/Settings/Personal/ServerDevNotice.php',
+        'OCA\\Settings\\SetupChecks\\CheckUserCertificates' => __DIR__ . '/..' . '/../lib/SetupChecks/CheckUserCertificates.php',
         'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat' => __DIR__ . '/..' . '/../lib/SetupChecks/LegacySSEKeyFormat.php',
         'OCA\\Settings\\SetupChecks\\PhpDefaultCharset' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpDefaultCharset.php',
         'OCA\\Settings\\SetupChecks\\PhpOutputBuffering' => __DIR__ . '/..' . '/../lib/SetupChecks/PhpOutputBuffering.php',
diff --git a/apps/settings/lib/AppInfo/Application.php b/apps/settings/lib/AppInfo/Application.php
index 2471400cb07..fe9ba63b012 100644
--- a/apps/settings/lib/AppInfo/Application.php
+++ b/apps/settings/lib/AppInfo/Application.php
@@ -98,16 +98,6 @@ class Application extends App implements IBootstrap {
 			}
 			return $isSubAdmin;
 		});
-		$context->registerService('userCertificateManager', function (IAppContainer $appContainer) {
-			/** @var IServerContainer $serverContainer */
-			$serverContainer = $appContainer->get(IServerContainer::class);
-			return $serverContainer->getCertificateManager();
-		}, false);
-		$context->registerService('systemCertificateManager', function (IAppContainer $appContainer) {
-			/** @var IServerContainer $serverContainer */
-			$serverContainer = $appContainer->query('ServerContainer');
-			return $serverContainer->getCertificateManager(null);
-		}, false);
 		$context->registerService(IProvider::class, function (IAppContainer $appContainer) {
 			/** @var IServerContainer $serverContainer */
 			$serverContainer = $appContainer->query(IServerContainer::class);
diff --git a/apps/settings/lib/Controller/CheckSetupController.php b/apps/settings/lib/Controller/CheckSetupController.php
index 76b97eb9dc4..0f9dd84febb 100644
--- a/apps/settings/lib/Controller/CheckSetupController.php
+++ b/apps/settings/lib/Controller/CheckSetupController.php
@@ -53,6 +53,7 @@ use OC\DB\SchemaWrapper;
 use OC\IntegrityCheck\Checker;
 use OC\Lock\NoopLockingProvider;
 use OC\MemoryInfo;
+use OCA\Settings\SetupChecks\CheckUserCertificates;
 use OCA\Settings\SetupChecks\LegacySSEKeyFormat;
 use OCA\Settings\SetupChecks\PhpDefaultCharset;
 use OCA\Settings\SetupChecks\PhpOutputBuffering;
@@ -692,6 +693,8 @@ Raw output
 		$phpDefaultCharset = new PhpDefaultCharset();
 		$phpOutputBuffering = new PhpOutputBuffering();
 		$legacySSEKeyFormat = new LegacySSEKeyFormat($this->l10n, $this->config, $this->urlGenerator);
+		$checkUserCertificates = new CheckUserCertificates($this->l10n, $this->config, $this->urlGenerator);
+
 		return new DataResponse(
 			[
 				'isGetenvServerWorking' => !empty(getenv('PATH')),
@@ -734,6 +737,7 @@ Raw output
 				PhpDefaultCharset::class => ['pass' => $phpDefaultCharset->run(), 'description' => $phpDefaultCharset->description(), 'severity' => $phpDefaultCharset->severity()],
 				PhpOutputBuffering::class => ['pass' => $phpOutputBuffering->run(), 'description' => $phpOutputBuffering->description(), 'severity' => $phpOutputBuffering->severity()],
 				LegacySSEKeyFormat::class => ['pass' => $legacySSEKeyFormat->run(), 'description' => $legacySSEKeyFormat->description(), 'severity' => $legacySSEKeyFormat->severity(), 'linkToDocumentation' => $legacySSEKeyFormat->linkToDocumentation()],
+				CheckUserCertificates::class => ['pass' => $checkUserCertificates->run(), 'description' => $checkUserCertificates->description(), 'severity' => $checkUserCertificates->severity(), 'elements' => $checkUserCertificates->elements()],
 			]
 		);
 	}
diff --git a/apps/settings/lib/SetupChecks/CheckUserCertificates.php b/apps/settings/lib/SetupChecks/CheckUserCertificates.php
new file mode 100644
index 00000000000..cbe6c91996a
--- /dev/null
+++ b/apps/settings/lib/SetupChecks/CheckUserCertificates.php
@@ -0,0 +1,80 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2020 Morris Jobke <hey@morrisjobke.de>
+ *
+ * @author Morris Jobke <hey@morrisjobke.de>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+namespace OCA\Settings\SetupChecks;
+
+use OCP\IConfig;
+use OCP\IL10N;
+use OCP\IURLGenerator;
+
+class CheckUserCertificates {
+	/** @var IL10N */
+	private $l10n;
+	/** @var string */
+	private $configValue;
+	/** @var IURLGenerator */
+	private $urlGenerator;
+
+	public function __construct(IL10N $l10n, IConfig $config, IURLGenerator $urlGenerator) {
+		$this->l10n = $l10n;
+		$configValue = $config->getAppValue('files_external', 'user_certificate_scan', false);
+		if (!is_string($configValue)) {
+			$configValue = '';
+		}
+		$this->configValue = $configValue;
+		$this->urlGenerator = $urlGenerator;
+	}
+
+	public function description(): string {
+		if ($this->configValue === '') {
+			return '';
+		}
+		if ($this->configValue === 'not-run-yet') {
+			return $this->l10n->t('A background job is pending that checks for user imported SSL certificates. Please check back later.');
+		}
+		return $this->l10n->t('There are some user imported SSL certificates present, that are not used anymore with Nextcloud 21. They can be imported on the command line via "occ security:certificates:import" command. Their paths inside the data directory are shown below.');
+	}
+
+	public function severity(): string {
+		return 'warning';
+	}
+
+	public function run(): bool {
+		// all fine if neither "not-run-yet" nor a result
+		return $this->configValue === '';
+	}
+
+	public function elements(): array {
+		if ($this->configValue === '' || $this->configValue === 'not-run-yet') {
+			return [];
+		}
+		$data = json_decode($this->configValue);
+		if (!is_array($data)) {
+			return [];
+		}
+		return $data;
+	}
+}
diff --git a/apps/settings/tests/Controller/CheckSetupControllerTest.php b/apps/settings/tests/Controller/CheckSetupControllerTest.php
index 64d27dcda89..dbc3fc7d0db 100644
--- a/apps/settings/tests/Controller/CheckSetupControllerTest.php
+++ b/apps/settings/tests/Controller/CheckSetupControllerTest.php
@@ -383,21 +383,25 @@ class CheckSetupControllerTest extends TestCase {
 	public function testCheck() {
 		$this->config->expects($this->at(0))
 			->method('getAppValue')
+			->with('files_external', 'user_certificate_scan', false)
+			->willReturn('["a", "b"]');
+		$this->config->expects($this->at(1))
+			->method('getAppValue')
 			->with('core', 'cronErrors')
 			->willReturn('');
-		$this->config->expects($this->at(2))
+		$this->config->expects($this->at(3))
 			->method('getSystemValue')
 			->with('connectivity_check_domains', ['www.nextcloud.com', 'www.startpage.com', 'www.eff.org', 'www.edri.org'])
 			->willReturn(['www.nextcloud.com', 'www.startpage.com', 'www.eff.org', 'www.edri.org']);
-		$this->config->expects($this->at(3))
+		$this->config->expects($this->at(4))
 			->method('getSystemValue')
 			->with('memcache.local', null)
 			->willReturn('SomeProvider');
-		$this->config->expects($this->at(4))
+		$this->config->expects($this->at(5))
 			->method('getSystemValue')
 			->with('has_internet_connection', true)
 			->willReturn(true);
-		$this->config->expects($this->at(5))
+		$this->config->expects($this->at(6))
 			->method('getSystemValue')
 			->with('appstoreenabled', true)
 			->willReturn(false);
@@ -594,6 +598,7 @@ class CheckSetupControllerTest extends TestCase {
 				'OCA\Settings\SetupChecks\PhpDefaultCharset' => ['pass' => true, 'description' => 'PHP configuration option default_charset should be UTF-8', 'severity' => 'warning'],
 				'OCA\Settings\SetupChecks\PhpOutputBuffering' => ['pass' => true, 'description' => 'PHP configuration option output_buffering must be disabled', 'severity' => 'error'],
 				'OCA\Settings\SetupChecks\LegacySSEKeyFormat' => ['pass' => true, 'description' => 'The old server-side-encryption format is enabled. We recommend disabling this.', 'severity' => 'warning', 'linkToDocumentation' => ''],
+				'OCA\Settings\SetupChecks\CheckUserCertificates' => ['pass' => false, 'description' => 'There are some user imported SSL certificates present, that are not used anymore with Nextcloud 21. They can be imported on the command line via "occ security:certificates:import" command. Their paths inside the data directory are shown below.', 'severity' => 'warning', 'elements' => ['a', 'b']],
 				'imageMagickLacksSVGSupport' => false,
 			]
 		);