diff options
-rw-r--r-- | apps/dav/lib/CardDAV/SyncService.php | 4 | ||||
-rw-r--r-- | apps/files_sharing/lib/External/Manager.php | 2 | ||||
-rw-r--r-- | apps/files_sharing/lib/External/MountProvider.php | 2 | ||||
-rw-r--r-- | core/register_command.php | 6 | ||||
-rw-r--r-- | lib/private/Files/Storage/DAV.php | 3 | ||||
-rw-r--r-- | lib/private/Security/CertificateManager.php | 66 | ||||
-rw-r--r-- | lib/private/Server.php | 44 | ||||
-rw-r--r-- | lib/public/ICertificateManager.php | 14 | ||||
-rw-r--r-- | lib/public/IServerContainer.php | 7 | ||||
-rw-r--r-- | tests/lib/Http/Client/ClientTest.php | 6 | ||||
-rw-r--r-- | tests/lib/Security/CertificateManagerTest.php | 61 | ||||
-rw-r--r-- | tests/lib/ServerTest.php | 4 |
12 files changed, 61 insertions, 158 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/core/register_command.php b/core/register_command.php index 840c73484bf..af6bd677251 100644 --- a/core/register_command.php +++ b/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)); diff --git a/lib/private/Files/Storage/DAV.php b/lib/private/Files/Storage/DAV.php index a6cfd77d98a..974feee8995 100644 --- a/lib/private/Files/Storage/DAV.php +++ b/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, '/'); diff --git a/lib/private/Security/CertificateManager.php b/lib/private/Security/CertificateManager.php index e69132ff4df..ed873527d3c 100644 --- a/lib/private/Security/CertificateManager.php +++ b/lib/private/Security/CertificateManager.php @@ -40,11 +40,6 @@ use OCP\Security\ISecureRandom; */ class CertificateManager implements ICertificateManager { /** - * @var string - */ - protected $uid; - - /** * @var \OC\Files\View */ protected $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; @@ -148,7 +140,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 +199,45 @@ 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; - } - 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(); - } + public function getAbsoluteBundlePath() { + 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); } diff --git a/lib/private/Server.php b/lib/private/Server.php index 5de7808523e..8e2452b2f30 100644 --- a/lib/private/Server.php +++ b/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); } /** diff --git a/lib/public/ICertificateManager.php b/lib/public/ICertificateManager.php index 1c7949a89cf..da97dc105d0 100644 --- a/lib/public/ICertificateManager.php +++ b/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(); } diff --git a/lib/public/IServerContainer.php b/lib/public/IServerContainer.php index 215cfab84fd..ed08b66b994 100644 --- a/lib/public/IServerContainer.php +++ b/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 diff --git a/tests/lib/Http/Client/ClientTest.php b/tests/lib/Http/Client/ClientTest.php index a462a0848ae..78054725e8c 100644 --- a/tests/lib/Http/Client/ClientTest.php +++ b/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([ diff --git a/tests/lib/Security/CertificateManagerTest.php b/tests/lib/Security/CertificateManagerTest.php index 447ee8719c9..3af4b564612 100644 --- a/tests/lib/Security/CertificateManagerTest.php +++ b/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 - - // 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], + //values: CaBundleMtime, targetBundleMtime, targetBundleExists, expected - // 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], ]; } } diff --git a/tests/lib/ServerTest.php b/tests/lib/ServerTest.php index ada25247393..1054c4195b2 100644 --- a/tests/lib/ServerTest.php +++ b/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() { |