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

Improve CertificateManager to not be user context dependent

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;

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);
 	}
 

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);
 	}

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',

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',

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);

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()],
 			]
 		);
 	}

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;
+	}
+}

apps/settings/tests/Controller/CheckSetupControllerTest.php

@@ -382,22 +382,26 @@ 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,
 			]
 		);

core/BackgroundJobs/CheckForUserCertificates.php

@@ -0,0 +1,79 @@
+<?php
+/**
+ * @copyright 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 OC\Core\BackgroundJobs;
+
+use OC\BackgroundJob\QueuedJob;
+use OCP\Files\Folder;
+use OCP\Files\IRootFolder;
+use OCP\Files\NotFoundException;
+use OCP\IConfig;
+use OCP\IUser;
+use OCP\IUserManager;
+
+class CheckForUserCertificates extends QueuedJob {
+
+	/** @var IConfig */
+	protected $config;
+	/** @var IUserManager */
+	private $userManager;
+	/** @var IRootFolder */
+	private $rootFolder;
+
+	public function __construct(IConfig $config, IUserManager $userManager, IRootFolder $rootFolder) {
+		$this->config = $config;
+		$this->userManager = $userManager;
+		$this->rootFolder = $rootFolder;
+	}
+
+	/**
+	 * Checks all user directories for old user uploaded certificates
+	 */
+	public function run($arguments) {
+		$uploadList = [];
+		$this->userManager->callForSeenUsers(function (IUser $user) use (&$uploadList) {
+			$userId = $user->getUID();
+			try {
+				\OC_Util::setupFS($userId);
+				$filesExternalUploadsFolder = $this->rootFolder->get($userId . '/files_external/uploads');
+			} catch (NotFoundException $e) {
+				\OC_Util::tearDownFS();
+				return;
+			}
+			if ($filesExternalUploadsFolder instanceof Folder) {
+				$files = $filesExternalUploadsFolder->getDirectoryListing();
+				foreach ($files as $file) {
+					$filename = $file->getName();
+					$uploadList[] = "$userId/files_external/uploads/$filename";
+				}
+			}
+			\OC_Util::tearDownFS();
+		});
+
+		if (empty($uploadList)) {
+			$this->config->deleteAppValue('files_external', 'user_certificate_scan');
+		} else {
+			$this->config->setAppValue('files_external', 'user_certificate_scan', json_encode($uploadList));
+		}
+	}
+}

core/js/setupchecks.js

@@ -488,6 +488,7 @@
 					OC.SetupChecks.addGenericSetupCheck(data, 'OCA\\Settings\\SetupChecks\\PhpDefaultCharset', messages)
 					OC.SetupChecks.addGenericSetupCheck(data, 'OCA\\Settings\\SetupChecks\\PhpOutputBuffering', messages)
 					OC.SetupChecks.addGenericSetupCheck(data, 'OCA\\Settings\\SetupChecks\\LegacySSEKeyFormat', messages)
+					OC.SetupChecks.addGenericSetupCheck(data, 'OCA\\Settings\\SetupChecks\\CheckUserCertificates', messages)
 
 				} else {
 					messages.push({
@@ -520,6 +521,15 @@
 			if (setupCheck.linkToDocumentation) {
 				message += ' ' + t('core', 'For more details see the <a target="_blank" rel="noreferrer noopener" href="{docLink}">documentation</a>.', {docLink: setupCheck.linkToDocumentation});
 			}
+			if (setupCheck.elements) {
+				message += '<br><ul>'
+				setupCheck.elements.forEach(function(element){
+					message += '<li>';
+					message += element
+					message += '</li>';
+				});
+				message += '</ul>'
+			}
 
 			if (!setupCheck.pass) {
 				messages.push({

core/register_command.php

@@ -183,9 +183,9 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) {
 	$application->add(new OC\Core\Command\Group\AddUser(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
 	$application->add(new OC\Core\Command\Group\RemoveUser(\OC::$server->getUserManager(), \OC::$server->getGroupManager()));
 
-	$application->add(new OC\Core\Command\Security\ListCertificates(\OC::$server->getCertificateManager(null), \OC::$server->getL10N('core')));
-	$application->add(new OC\Core\Command\Security\ImportCertificate(\OC::$server->getCertificateManager(null)));
-	$application->add(new OC\Core\Command\Security\RemoveCertificate(\OC::$server->getCertificateManager(null)));
+	$application->add(new OC\Core\Command\Security\ListCertificates(\OC::$server->getCertificateManager(), \OC::$server->getL10N('core')));
+	$application->add(new OC\Core\Command\Security\ImportCertificate(\OC::$server->getCertificateManager()));
+	$application->add(new OC\Core\Command\Security\RemoveCertificate(\OC::$server->getCertificateManager()));
 	$application->add(new OC\Core\Command\Security\ResetBruteforceAttempts(\OC::$server->getBruteForceThrottler()));
 } else {
 	$application->add(\OC::$server->get(\OC\Core\Command\Maintenance\Install::class));

lib/composer/composer/autoload_classmap.php

@@ -769,6 +769,7 @@ return array(
     'OC\\Contacts\\ContactsMenu\\Providers\\EMailProvider' => $baseDir . '/lib/private/Contacts/ContactsMenu/Providers/EMailProvider.php',
     'OC\\Core\\Application' => $baseDir . '/core/Application.php',
     'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => $baseDir . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php',
+    'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => $baseDir . '/core/BackgroundJobs/CheckForUserCertificates.php',
     'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => $baseDir . '/core/BackgroundJobs/CleanupLoginFlowV2.php',
     'OC\\Core\\Command\\App\\CheckCode' => $baseDir . '/core/Command/App/CheckCode.php',
     'OC\\Core\\Command\\App\\Disable' => $baseDir . '/core/Command/App/Disable.php',
@@ -1254,6 +1255,7 @@ return array(
     'OC\\Repair\\NC20\\EncryptionLegacyCipher' => $baseDir . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php',
     'OC\\Repair\\NC20\\EncryptionMigration' => $baseDir . '/lib/private/Repair/NC20/EncryptionMigration.php',
     'OC\\Repair\\NC20\\ShippedDashboardEnable' => $baseDir . '/lib/private/Repair/NC20/ShippedDashboardEnable.php',
+    'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => $baseDir . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php',
     'OC\\Repair\\OldGroupMembershipShares' => $baseDir . '/lib/private/Repair/OldGroupMembershipShares.php',
     'OC\\Repair\\Owncloud\\DropAccountTermsTable' => $baseDir . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
     'OC\\Repair\\Owncloud\\SaveAccountsTableData' => $baseDir . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',

lib/composer/composer/autoload_static.php

@@ -798,6 +798,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Contacts\\ContactsMenu\\Providers\\EMailProvider' => __DIR__ . '/../../..' . '/lib/private/Contacts/ContactsMenu/Providers/EMailProvider.php',
         'OC\\Core\\Application' => __DIR__ . '/../../..' . '/core/Application.php',
         'OC\\Core\\BackgroundJobs\\BackgroundCleanupUpdaterBackupsJob' => __DIR__ . '/../../..' . '/core/BackgroundJobs/BackgroundCleanupUpdaterBackupsJob.php',
+        'OC\\Core\\BackgroundJobs\\CheckForUserCertificates' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CheckForUserCertificates.php',
         'OC\\Core\\BackgroundJobs\\CleanupLoginFlowV2' => __DIR__ . '/../../..' . '/core/BackgroundJobs/CleanupLoginFlowV2.php',
         'OC\\Core\\Command\\App\\CheckCode' => __DIR__ . '/../../..' . '/core/Command/App/CheckCode.php',
         'OC\\Core\\Command\\App\\Disable' => __DIR__ . '/../../..' . '/core/Command/App/Disable.php',
@@ -1283,6 +1284,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Repair\\NC20\\EncryptionLegacyCipher' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionLegacyCipher.php',
         'OC\\Repair\\NC20\\EncryptionMigration' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/EncryptionMigration.php',
         'OC\\Repair\\NC20\\ShippedDashboardEnable' => __DIR__ . '/../../..' . '/lib/private/Repair/NC20/ShippedDashboardEnable.php',
+        'OC\\Repair\\NC21\\AddCheckForUserCertificatesJob' => __DIR__ . '/../../..' . '/lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php',
         'OC\\Repair\\OldGroupMembershipShares' => __DIR__ . '/../../..' . '/lib/private/Repair/OldGroupMembershipShares.php',
         'OC\\Repair\\Owncloud\\DropAccountTermsTable' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/DropAccountTermsTable.php',
         'OC\\Repair\\Owncloud\\SaveAccountsTableData' => __DIR__ . '/../../..' . '/lib/private/Repair/Owncloud/SaveAccountsTableData.php',

lib/private/Files/Storage/DAV.php

@@ -122,9 +122,6 @@ class DAV extends Common {
 			if ($this->secure === true) {
 				// inject mock for testing
 				$this->certManager = \OC::$server->getCertificateManager();
-				if (is_null($this->certManager)) { //no user
-					$this->certManager = \OC::$server->getCertificateManager(null);
-				}
 			}
 			$this->root = $params['root'] ?? '/';
 			$this->root = '/' . ltrim($this->root, '/');

lib/private/Http/Client/Client.php

@@ -105,10 +105,6 @@ class Client implements IClient {
 			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
 		}
 
-		if ($this->certificateManager->listCertificates() === []) {
-			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
-		}
-
 		return $this->certificateManager->getAbsoluteBundlePath();
 	}
 

lib/private/Repair.php

@@ -52,6 +52,7 @@ use OC\Repair\NC18\ResetGeneratedAvatarFlag;
 use OC\Repair\NC20\EncryptionLegacyCipher;
 use OC\Repair\NC20\EncryptionMigration;
 use OC\Repair\NC20\ShippedDashboardEnable;
+use OC\Repair\NC21\AddCheckForUserCertificatesJob;
 use OC\Repair\OldGroupMembershipShares;
 use OC\Repair\Owncloud\DropAccountTermsTable;
 use OC\Repair\Owncloud\SaveAccountsTableData;
@@ -164,6 +165,7 @@ class Repair implements IOutput {
 			\OC::$server->query(EncryptionMigration::class),
 			\OC::$server->get(ShippedDashboardEnable::class),
 			\OC::$server->get(AddBruteForceCleanupJob::class),
+			\OC::$server->get(AddCheckForUserCertificatesJob::class),
 		];
 	}
 

lib/private/Repair/NC21/AddCheckForUserCertificatesJob.php

@@ -0,0 +1,61 @@
+<?php
+/**
+ * @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 OC\Repair\NC21;
+
+use OC\Core\BackgroundJobs\CheckForUserCertificates;
+use OCP\BackgroundJob\IJobList;
+use OCP\IConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+class AddCheckForUserCertificatesJob implements IRepairStep {
+
+	/** @var IJobList */
+	protected $jobList;
+	/** @var IConfig */
+	private $config;
+
+	public function __construct(IConfig $config, IJobList $jobList) {
+		$this->jobList = $jobList;
+		$this->config = $config;
+	}
+
+	public function getName() {
+		return 'Queue a one-time job to check for user uploaded certificates';
+	}
+
+	private function shouldRun() {
+		$versionFromBeforeUpdate = $this->config->getSystemValue('version', '0.0.0.0');
+
+		// was added to 21.0.0.2
+		return version_compare($versionFromBeforeUpdate, '21.0.0.2', '<');
+	}
+
+	public function run(IOutput $output) {
+		if ($this->shouldRun()) {
+			$this->config->setAppValue('files_external', 'user_certificate_scan', 'not-run-yet');
+			$this->jobList->add(CheckForUserCertificates::class);
+		}
+	}
+}

lib/private/Security/CertificateManager.php

@@ -39,11 +39,6 @@ use OCP\Security\ISecureRandom;
  * Manage trusted certificates for users
  */
 class CertificateManager implements ICertificateManager {
-	/**
-	 * @var string
-	 */
-	protected $uid;
-
 	/**
 	 * @var \OC\Files\View
 	 */
@@ -63,18 +58,15 @@ class CertificateManager implements ICertificateManager {
 	protected $random;
 
 	/**
-	 * @param string $uid
 	 * @param \OC\Files\View $view relative to data/
 	 * @param IConfig $config
 	 * @param ILogger $logger
 	 * @param ISecureRandom $random
 	 */
-	public function __construct($uid,
-								\OC\Files\View $view,
+	public function __construct(\OC\Files\View $view,
 								IConfig $config,
 								ILogger $logger,
 								ISecureRandom $random) {
-		$this->uid = $uid;
 		$this->view = $view;
 		$this->config = $config;
 		$this->logger = $logger;
@@ -112,6 +104,29 @@ class CertificateManager implements ICertificateManager {
 		return $result;
 	}
 
+	private function hasCertificates(): bool {
+		if (!$this->config->getSystemValue('installed', false)) {
+			return false;
+		}
+
+		$path = $this->getPathToCertificates() . 'uploads/';
+		if (!$this->view->is_dir($path)) {
+			return false;
+		}
+		$result = [];
+		$handle = $this->view->opendir($path);
+		if (!is_resource($handle)) {
+			return false;
+		}
+		while (false !== ($file = readdir($handle))) {
+			if ($file !== '.' && $file !== '..') {
+				return true;
+			}
+		}
+		closedir($handle);
+		return false;
+	}
+
 	/**
 	 * create the certificate bundle of all trusted certificated
 	 */
@@ -148,7 +163,7 @@ class CertificateManager implements ICertificateManager {
 		fwrite($fhCerts, $defaultCertificates);
 
 		// Append the system certificate bundle
-		$systemBundle = $this->getCertificateBundle(null);
+		$systemBundle = $this->getCertificateBundle();
 		if ($systemBundle !== $certPath && $this->view->file_exists($systemBundle)) {
 			$systemCertificates = $this->view->file_get_contents($systemBundle);
 			fwrite($fhCerts, $systemCertificates);
@@ -207,73 +222,50 @@ class CertificateManager implements ICertificateManager {
 	}
 
 	/**
-	 * Get the path to the certificate bundle for this user
+	 * Get the path to the certificate bundle
 	 *
-	 * @param string|null $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 */
-	public function getCertificateBundle($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		return $this->getPathToCertificates($uid) . 'rootcerts.crt';
+	public function getCertificateBundle() {
+		return $this->getPathToCertificates() . 'rootcerts.crt';
 	}
 
 	/**
-	 * Get the full local path to the certificate bundle for this user
+	 * Get the full local path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 */
-	public function getAbsoluteBundlePath($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
+	public function getAbsoluteBundlePath() {
+		if (!$this->hasCertificates()) {
+			return \OC::$SERVERROOT . '/resources/config/ca-bundle.crt';
 		}
-		if ($this->needsRebundling($uid)) {
-			if (is_null($uid)) {
-				$manager = new CertificateManager(null, $this->view, $this->config, $this->logger, $this->random);
-				$manager->createCertificateBundle();
-			} else {
-				$this->createCertificateBundle();
-			}
+
+		if ($this->needsRebundling()) {
+			$this->createCertificateBundle();
 		}
-		return $this->view->getLocalFile($this->getCertificateBundle($uid));
+
+		return $this->view->getLocalFile($this->getCertificateBundle());
 	}
 
 	/**
-	 * @param string|null $uid (optional) user to get the certificate path for, use `null` to get the system path
 	 * @return string
 	 */
-	private function getPathToCertificates($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		return is_null($uid) ? '/files_external/' : '/' . $uid . '/files_external/';
+	private function getPathToCertificates() {
+		return '/files_external/';
 	}
 
 	/**
 	 * Check if we need to re-bundle the certificates because one of the sources has updated
 	 *
-	 * @param string $uid (optional) user to get the certificate path for, use `null` to get the system path
 	 * @return bool
 	 */
-	private function needsRebundling($uid = '') {
-		if ($uid === '') {
-			$uid = $this->uid;
-		}
-		$sourceMTimes = [$this->getFilemtimeOfCaBundle()];
-		$targetBundle = $this->getCertificateBundle($uid);
+	private function needsRebundling() {
+		$targetBundle = $this->getCertificateBundle();
 		if (!$this->view->file_exists($targetBundle)) {
 			return true;
 		}
 
-		if (!is_null($uid)) { // also depend on the system bundle
-			$sourceMTimes[] = $this->view->filemtime($this->getCertificateBundle(null));
-		}
-
-		$sourceMTime = array_reduce($sourceMTimes, function ($max, $mtime) {
-			return max($max, $mtime);
-		}, 0);
+		$sourceMTime = $this->getFilemtimeOfCaBundle();
 		return $sourceMTime > $this->view->filemtime($targetBundle);
 	}
 

lib/private/Server.php

@@ -178,6 +178,7 @@ use OCP\IAppConfig;
 use OCP\IAvatarManager;
 use OCP\ICache;
 use OCP\ICacheFactory;
+use OCP\ICertificateManager;
 use OCP\IDateTimeFormatter;
 use OCP\IDateTimeZone;
 use OCP\IDBConnection;
@@ -823,23 +824,8 @@ class Server extends ServerContainer implements IServerContainer {
 		/** @deprecated 19.0.0 */
 		$this->registerDeprecatedAlias('DatabaseConnection', IDBConnection::class);
 
-
-		$this->registerService(IClientService::class, function (ContainerInterface $c) {
-			$user = \OC_User::getUser();
-			$uid = $user ? $user : null;
-			return new ClientService(
-				$c->get(\OCP\IConfig::class),
-				$c->get(ILogger::class),
-				new \OC\Security\CertificateManager(
-					$uid,
-					new View(),
-					$c->get(\OCP\IConfig::class),
-					$c->get(ILogger::class),
-					$c->get(ISecureRandom::class)
-				)
-			);
-		});
-		/** @deprecated 19.0.0 */
+		$this->registerAlias(ICertificateManager::class, CertificateManager::class);
+		$this->registerAlias(IClientService::class, ClientService::class);
 		$this->registerDeprecatedAlias('HttpClientService', IClientService::class);
 		$this->registerService(IEventLogger::class, function (ContainerInterface $c) {
 			$eventLogger = new EventLogger();
@@ -1840,28 +1826,12 @@ class Server extends ServerContainer implements IServerContainer {
 	}
 
 	/**
-	 * Get the certificate manager for the user
+	 * Get the certificate manager
 	 *
-	 * @param string $userId (optional) if not specified the current loggedin user is used, use null to get the system certificate manager
-	 * @return \OCP\ICertificateManager | null if $uid is null and no user is logged in
-	 * @deprecated 20.0.0
+	 * @return \OCP\ICertificateManager
 	 */
-	public function getCertificateManager($userId = '') {
-		if ($userId === '') {
-			$userSession = $this->get(IUserSession::class);
-			$user = $userSession->getUser();
-			if (is_null($user)) {
-				return null;
-			}
-			$userId = $user->getUID();
-		}
-		return new CertificateManager(
-			$userId,
-			new View(),
-			$this->get(\OCP\IConfig::class),
-			$this->get(ILogger::class),
-			$this->get(ISecureRandom::class)
-		);
+	public function getCertificateManager() {
+		return $this->get(ICertificateManager::class);
 	}
 
 	/**

lib/public/ICertificateManager.php

@@ -25,12 +25,12 @@
 namespace OCP;
 
 /**
- * Manage trusted certificates for users
+ * Manage trusted certificates
  * @since 8.0.0
  */
 interface ICertificateManager {
 	/**
-	 * Returns all certificates trusted by the user
+	 * Returns all certificates trusted by the system
 	 *
 	 * @return \OCP\ICertificate[]
 	 * @since 8.0.0
@@ -53,20 +53,18 @@ interface ICertificateManager {
 	public function removeCertificate($name);
 
 	/**
-	 * Get the path to the certificate bundle for this user
+	 * Get the path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle (since 9.0.0)
 	 * @return string
 	 * @since 8.0.0
 	 */
-	public function getCertificateBundle($uid = '');
+	public function getCertificateBundle();
 
 	/**
-	 * Get the full local path to the certificate bundle for this user
+	 * Get the full local path to the certificate bundle
 	 *
-	 * @param string $uid (optional) user to get the certificate bundle for, use `null` to get the system bundle
 	 * @return string
 	 * @since 9.0.0
 	 */
-	public function getAbsoluteBundlePath($uid = '');
+	public function getAbsoluteBundlePath();
 }

lib/public/IServerContainer.php

@@ -387,14 +387,13 @@ interface IServerContainer extends ContainerInterface, IContainer {
 	public function getSearch();
 
 	/**
-	 * Get the certificate manager for the user
+	 * Get the certificate manager
 	 *
-	 * @param string $userId (optional) if not specified the current loggedin user is used, use null to get the system certificate manager
-	 * @return \OCP\ICertificateManager | null if $userId is null and no user is logged in
+	 * @return \OCP\ICertificateManager
 	 * @since 8.0.0
 	 * @deprecated 20.0.0 have it injected or fetch it through \Psr\Container\ContainerInterface::get
 	 */
-	public function getCertificateManager($userId = null);
+	public function getCertificateManager();
 
 	/**
 	 * Create a new event source

tests/lib/Http/Client/ClientTest.php

@@ -281,7 +281,7 @@ class ClientTest extends \Test\TestCase {
 		$this->certificateManager
 			->expects($this->once())
 			->method('getAbsoluteBundlePath')
-			->with(null)
+			->with()
 			->willReturn('/my/path.crt');
 
 		$this->defaultRequestOptions = [
@@ -493,7 +493,7 @@ class ClientTest extends \Test\TestCase {
 		$this->certificateManager
 			->expects($this->once())
 			->method('getAbsoluteBundlePath')
-			->with(null)
+			->with()
 			->willReturn('/my/path.crt');
 
 		$this->assertEquals([
@@ -529,7 +529,7 @@ class ClientTest extends \Test\TestCase {
 		$this->certificateManager
 			->expects($this->once())
 			->method('getAbsoluteBundlePath')
-			->with(null)
+			->with()
 			->willReturn('/my/path.crt');
 
 		$this->assertEquals([

tests/lib/Security/CertificateManagerTest.php

@@ -53,7 +53,6 @@ class CertificateManagerTest extends \Test\TestCase {
 			->willReturn('random');
 
 		$this->certificateManager = new CertificateManager(
-			$this->username,
 			new \OC\Files\View(),
 			$config,
 			$this->createMock(ILogger::class),
@@ -132,22 +131,18 @@ class CertificateManagerTest extends \Test\TestCase {
 	}
 
 	public function testGetCertificateBundle() {
-		$this->assertSame('/' . $this->username . '/files_external/rootcerts.crt', $this->certificateManager->getCertificateBundle());
+		$this->assertSame('/files_external/rootcerts.crt', $this->certificateManager->getCertificateBundle());
 	}
 
 	/**
 	 * @dataProvider dataTestNeedRebundling
 	 *
-	 * @param string $uid
 	 * @param int $CaBundleMtime
-	 * @param int $systemWideMtime
 	 * @param int $targetBundleMtime
 	 * @param int $targetBundleExists
 	 * @param bool $expected
 	 */
-	public function testNeedRebundling($uid,
-								$CaBundleMtime,
-								$systemWideMtime,
+	public function testNeedRebundling($CaBundleMtime,
 								$targetBundleMtime,
 								$targetBundleExists,
 								$expected
@@ -158,31 +153,24 @@ class CertificateManagerTest extends \Test\TestCase {
 
 		/** @var CertificateManager | \PHPUnit\Framework\MockObject\MockObject $certificateManager */
 		$certificateManager = $this->getMockBuilder('OC\Security\CertificateManager')
-			->setConstructorArgs([$uid, $view, $config, $this->createMock(ILogger::class), $this->random])
+			->setConstructorArgs([$view, $config, $this->createMock(ILogger::class), $this->random])
 			->setMethods(['getFilemtimeOfCaBundle', 'getCertificateBundle'])
 			->getMock();
 
 		$certificateManager->expects($this->any())->method('getFilemtimeOfCaBundle')
 			->willReturn($CaBundleMtime);
 
-		$certificateManager->expects($this->at(1))->method('getCertificateBundle')
-			->with($uid)->willReturn('targetBundlePath');
+		$certificateManager->expects($this->at(0))->method('getCertificateBundle')
+			->willReturn('targetBundlePath');
 
 		$view->expects($this->any())->method('file_exists')
 			->with('targetBundlePath')
 			->willReturn($targetBundleExists);
 
 
-		if ($uid !== null && $targetBundleExists) {
-			$certificateManager->expects($this->at(2))->method('getCertificateBundle')
-				->with(null)->willReturn('SystemBundlePath');
-		}
-
 		$view->expects($this->any())->method('filemtime')
-			->willReturnCallback(function ($path) use ($systemWideMtime, $targetBundleMtime) {
-				if ($path === 'SystemBundlePath') {
-					return $systemWideMtime;
-				} elseif ($path === 'targetBundlePath') {
+			->willReturnCallback(function ($path) use ($targetBundleMtime) {
+				if ($path === 'targetBundlePath') {
 					return $targetBundleMtime;
 				}
 				throw new \Exception('unexpected path');
@@ -190,37 +178,26 @@ class CertificateManagerTest extends \Test\TestCase {
 
 
 		$this->assertSame($expected,
-			$this->invokePrivate($certificateManager, 'needsRebundling', [$uid])
+			$this->invokePrivate($certificateManager, 'needsRebundling')
 		);
 	}
 
 	public function dataTestNeedRebundling() {
 		return [
-			//values: uid, CaBundleMtime, systemWideMtime, targetBundleMtime, targetBundleExists, expected
+			//values: CaBundleMtime, targetBundleMtime, targetBundleExists, expected
 
-			// compare minimum of CaBundleMtime and systemWideMtime with targetBundleMtime
-			['user1', 10, 20, 30, true, false],
-			['user1', 10, 20, 15, true, true],
-			['user1', 10, 5, 30, true, false],
-			['user1', 10, 5, 8, true, true],
-
-			// if no user exists we ignore 'systemWideMtime' because this is the bundle we re-build
-			[null, 10, 20, 30, true, false],
-			[null, 10, 20, 15, true, false],
-			[null, 10, 20, 8, true, true],
-			[null, 10, 5, 30, true, false],
-			[null, 10, 5, 8, true, true],
+			[10, 30, true, false],
+			[10, 15, true, false],
+			[10, 8, true, true],
+			[10, 30, true, false],
+			[10, 8, true, true],
 
 			// if no target bundle exists we always build a new one
-			['user1', 10, 20, 30, false, true],
-			['user1', 10, 20, 15, false, true],
-			['user1', 10, 5, 30, false, true],
-			['user1', 10, 5, 8, false, true],
-			[null, 10, 20, 30, false, true],
-			[null, 10, 20, 15, false, true],
-			[null, 10, 20, 8, false, true],
-			[null, 10, 5, 30, false, true],
-			[null, 10, 5, 8, false, true],
+			[10, 30, false, true],
+			[10, 15, false, true],
+			[10, 8, false, true],
+			[10, 30, false, true],
+			[10, 8, false, true],
 		];
 	}
 }

tests/lib/ServerTest.php

@@ -175,8 +175,8 @@ class ServerTest extends \Test\TestCase {
 	}
 
 	public function testGetCertificateManager() {
-		$this->assertInstanceOf('\OC\Security\CertificateManager', $this->server->getCertificateManager('test'), 'service returned by "getCertificateManager" did not return the right class');
-		$this->assertInstanceOf('\OCP\ICertificateManager', $this->server->getCertificateManager('test'), 'service returned by "getCertificateManager" did not return the right class');
+		$this->assertInstanceOf('\OC\Security\CertificateManager', $this->server->getCertificateManager(), 'service returned by "getCertificateManager" did not return the right class');
+		$this->assertInstanceOf('\OCP\ICertificateManager', $this->server->getCertificateManager(), 'service returned by "getCertificateManager" did not return the right class');
 	}
 
 	public function testCreateEventSource() {

version.php

@@ -29,7 +29,7 @@
 // between betas, final and RCs. This is _not_ the public version number. Reset minor/patchlevel
 // when updating major/minor version number.
 
-$OC_Version = [21, 0, 0, 1];
+$OC_Version = [21, 0, 0, 2];
 
 // The human readable string
 $OC_VersionString = '21.0.0 alpha';