diff options
author | Roeland Jago Douma <rullzer@users.noreply.github.com> | 2020-06-07 10:42:19 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-06-07 10:42:19 +0200 |
commit | 8325ab4bf05494f8c54d226784a55c0988e30f88 (patch) | |
tree | 02a7c298db31dfc50f417534b80e8b1e881c3d82 | |
parent | e212908cece6bd9128f3a80fa1334b0d3464c61b (diff) | |
parent | 817eff6cbf7cc777c38f76e849b3e94d79bfdb5f (diff) | |
download | nextcloud-server-8325ab4bf05494f8c54d226784a55c0988e30f88.tar.gz nextcloud-server-8325ab4bf05494f8c54d226784a55c0988e30f88.zip |
Merge pull request #21224 from nextcloud/techdebt/noid/remove-unreachable-controller-and-route
Remove controller and routes which have no UI component anyway
-rw-r--r-- | apps/settings/appinfo/routes.php | 4 | ||||
-rw-r--r-- | apps/settings/composer/composer/autoload_classmap.php | 1 | ||||
-rw-r--r-- | apps/settings/composer/composer/autoload_static.php | 1 | ||||
-rw-r--r-- | apps/settings/lib/Controller/CertificateController.php | 177 | ||||
-rw-r--r-- | apps/settings/templates/certificates.php | 44 | ||||
-rw-r--r-- | apps/settings/tests/AppInfo/ApplicationTest.php | 32 | ||||
-rw-r--r-- | apps/settings/tests/Controller/CertificateControllerTest.php | 196 |
7 files changed, 0 insertions, 455 deletions
diff --git a/apps/settings/appinfo/routes.php b/apps/settings/appinfo/routes.php index d8a4b2aabd6..b6d68d76d64 100644 --- a/apps/settings/appinfo/routes.php +++ b/apps/settings/appinfo/routes.php @@ -66,10 +66,6 @@ return [ ['name' => 'CheckSetup#check', 'url' => '/settings/ajax/checksetup', 'verb' => 'GET' , 'root' => ''], ['name' => 'CheckSetup#getFailedIntegrityCheckFiles', 'url' => '/settings/integrity/failed', 'verb' => 'GET' , 'root' => ''], ['name' => 'CheckSetup#rescanFailedIntegrityCheck', 'url' => '/settings/integrity/rescan', 'verb' => 'GET' , 'root' => ''], - ['name' => 'Certificate#addPersonalRootCertificate', 'url' => '/settings/personal/certificate', 'verb' => 'POST' , 'root' => ''], - ['name' => 'Certificate#removePersonalRootCertificate', 'url' => '/settings/personal/certificate/{certificateIdentifier}', 'verb' => 'DELETE' , 'root' => ''], - ['name' => 'Certificate#addSystemRootCertificate', 'url' => '/settings/admin/certificate', 'verb' => 'POST' , 'root' => ''], - ['name' => 'Certificate#removeSystemRootCertificate', 'url' => '/settings/admin/certificate/{certificateIdentifier}', 'verb' => 'DELETE' , 'root' => ''], ['name' => 'PersonalSettings#index', 'url' => '/settings/user/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'personal-info'] , 'root' => ''], ['name' => 'AdminSettings#index', 'url' => '/settings/admin/{section}', 'verb' => 'GET', 'defaults' => ['section' => 'server'] , 'root' => ''], ['name' => 'AdminSettings#form', 'url' => '/settings/admin/{section}', 'verb' => 'GET' , 'root' => ''], diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 9d89ec1a999..ce594d9b46d 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -18,7 +18,6 @@ return array( 'OCA\\Settings\\Controller\\AdminSettingsController' => $baseDir . '/../lib/Controller/AdminSettingsController.php', 'OCA\\Settings\\Controller\\AppSettingsController' => $baseDir . '/../lib/Controller/AppSettingsController.php', 'OCA\\Settings\\Controller\\AuthSettingsController' => $baseDir . '/../lib/Controller/AuthSettingsController.php', - 'OCA\\Settings\\Controller\\CertificateController' => $baseDir . '/../lib/Controller/CertificateController.php', 'OCA\\Settings\\Controller\\ChangePasswordController' => $baseDir . '/../lib/Controller/ChangePasswordController.php', 'OCA\\Settings\\Controller\\CheckSetupController' => $baseDir . '/../lib/Controller/CheckSetupController.php', 'OCA\\Settings\\Controller\\CommonSettingsTrait' => $baseDir . '/../lib/Controller/CommonSettingsTrait.php', diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index c1c1abb189f..b77b581a0fc 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -33,7 +33,6 @@ class ComposerStaticInitSettings 'OCA\\Settings\\Controller\\AdminSettingsController' => __DIR__ . '/..' . '/../lib/Controller/AdminSettingsController.php', 'OCA\\Settings\\Controller\\AppSettingsController' => __DIR__ . '/..' . '/../lib/Controller/AppSettingsController.php', 'OCA\\Settings\\Controller\\AuthSettingsController' => __DIR__ . '/..' . '/../lib/Controller/AuthSettingsController.php', - 'OCA\\Settings\\Controller\\CertificateController' => __DIR__ . '/..' . '/../lib/Controller/CertificateController.php', 'OCA\\Settings\\Controller\\ChangePasswordController' => __DIR__ . '/..' . '/../lib/Controller/ChangePasswordController.php', 'OCA\\Settings\\Controller\\CheckSetupController' => __DIR__ . '/..' . '/../lib/Controller/CheckSetupController.php', 'OCA\\Settings\\Controller\\CommonSettingsTrait' => __DIR__ . '/..' . '/../lib/Controller/CommonSettingsTrait.php', diff --git a/apps/settings/lib/Controller/CertificateController.php b/apps/settings/lib/Controller/CertificateController.php deleted file mode 100644 index b7ce1749660..00000000000 --- a/apps/settings/lib/Controller/CertificateController.php +++ /dev/null @@ -1,177 +0,0 @@ -<?php -/** - * @copyright Copyright (c) 2016, ownCloud, Inc. - * - * @author Björn Schießle <bjoern@schiessle.org> - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * @author Lukas Reschke <lukas@statuscode.ch> - * @author Robin Appelman <robin@icewind.nl> - * @author Roeland Jago Douma <roeland@famdouma.nl> - * @author Vincent Petry <pvince81@owncloud.com> - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * 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, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ - -namespace OCA\Settings\Controller; - -use OCP\App\IAppManager; -use OCP\AppFramework\Controller; -use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; -use OCP\ICertificateManager; -use OCP\IL10N; -use OCP\IRequest; - -class CertificateController extends Controller { - /** @var ICertificateManager */ - private $userCertificateManager; - /** @var ICertificateManager */ - private $systemCertificateManager; - /** @var IL10N */ - private $l10n; - /** @var IAppManager */ - private $appManager; - - /** - * @param string $appName - * @param IRequest $request - * @param ICertificateManager $userCertificateManager - * @param ICertificateManager $systemCertificateManager - * @param IL10N $l10n - * @param IAppManager $appManager - */ - public function __construct($appName, - IRequest $request, - ICertificateManager $userCertificateManager, - ICertificateManager $systemCertificateManager, - IL10N $l10n, - IAppManager $appManager) { - parent::__construct($appName, $request); - $this->userCertificateManager = $userCertificateManager; - $this->systemCertificateManager = $systemCertificateManager; - $this->l10n = $l10n; - $this->appManager = $appManager; - } - - /** - * Add a new personal root certificate to the users' trust store - * - * @NoAdminRequired - * @NoSubadminRequired - * @return DataResponse - */ - public function addPersonalRootCertificate() { - return $this->addCertificate($this->userCertificateManager); - } - - /** - * Add a new root certificate to a trust store - * - * @param ICertificateManager $certificateManager - * @return DataResponse - */ - private function addCertificate(ICertificateManager $certificateManager) { - $headers = []; - - if ($this->isCertificateImportAllowed() === false) { - return new DataResponse(['message' => 'Individual certificate management disabled'], Http::STATUS_FORBIDDEN, $headers); - } - - $file = $this->request->getUploadedFile('rootcert_import'); - if (empty($file)) { - return new DataResponse(['message' => 'No file uploaded'], Http::STATUS_UNPROCESSABLE_ENTITY, $headers); - } - - try { - $certificate = $certificateManager->addCertificate(file_get_contents($file['tmp_name']), $file['name']); - return new DataResponse( - [ - 'name' => $certificate->getName(), - 'commonName' => $certificate->getCommonName(), - 'organization' => $certificate->getOrganization(), - 'validFrom' => $certificate->getIssueDate()->getTimestamp(), - 'validTill' => $certificate->getExpireDate()->getTimestamp(), - 'validFromString' => $this->l10n->l('date', $certificate->getIssueDate()), - 'validTillString' => $this->l10n->l('date', $certificate->getExpireDate()), - 'issuer' => $certificate->getIssuerName(), - 'issuerOrganization' => $certificate->getIssuerOrganization(), - ], - Http::STATUS_OK, - $headers - ); - } catch (\Exception $e) { - return new DataResponse(['An error occurred.'], Http::STATUS_UNPROCESSABLE_ENTITY, $headers); - } - } - - /** - * Removes a personal root certificate from the users' trust store - * - * @NoAdminRequired - * @NoSubadminRequired - * @param string $certificateIdentifier - * @return DataResponse - */ - public function removePersonalRootCertificate($certificateIdentifier) { - if ($this->isCertificateImportAllowed() === false) { - return new DataResponse(['Individual certificate management disabled'], Http::STATUS_FORBIDDEN); - } - - $this->userCertificateManager->removeCertificate($certificateIdentifier); - return new DataResponse(); - } - - /** - * check if certificate import is allowed - * - * @return bool - */ - protected function isCertificateImportAllowed() { - $externalStorageEnabled = $this->appManager->isEnabledForUser('files_external'); - if ($externalStorageEnabled) { - /** @var \OCA\Files_External\Service\BackendService $backendService */ - $backendService = \OC_Mount_Config::$app->getContainer()->query('\OCA\Files_External\Service\BackendService'); - if ($backendService->isUserMountingAllowed()) { - return true; - } - } - return false; - } - - /** - * Add a new personal root certificate to the system's trust store - * - * @return DataResponse - */ - public function addSystemRootCertificate() { - return $this->addCertificate($this->systemCertificateManager); - } - - /** - * Removes a personal root certificate from the users' trust store - * - * @param string $certificateIdentifier - * @return DataResponse - */ - public function removeSystemRootCertificate($certificateIdentifier) { - if ($this->isCertificateImportAllowed() === false) { - return new DataResponse(['Individual certificate management disabled'], Http::STATUS_FORBIDDEN); - } - - $this->systemCertificateManager->removeCertificate($certificateIdentifier); - return new DataResponse(); - } -} diff --git a/apps/settings/templates/certificates.php b/apps/settings/templates/certificates.php deleted file mode 100644 index d9587f97946..00000000000 --- a/apps/settings/templates/certificates.php +++ /dev/null @@ -1,44 +0,0 @@ -<div class="section"> - <h2 data-anchor-name="ssl-root-certificate"><?php p($l->t('SSL Root Certificates')); ?></h2> - <table id="sslCertificate" class="grid" data-type="<?php p($_['type']); ?>"> - <thead> - <tr> - <th><?php p($l->t('Common Name')); ?></th> - <th><?php p($l->t('Valid until')); ?></th> - <th><?php p($l->t('Issued By')); ?></th> - </tr> - </thead> - <tbody> - <?php foreach ($_['certs'] as $rootCert): /**@var \OCP\ICertificate $rootCert */ ?> - <tr class="<?php echo $rootCert->isExpired() ? 'expired' : 'valid' ?>" - data-name="<?php p($rootCert->getName()) ?>"> - <td class="rootCert" - title="<?php p($rootCert->getOrganization()) ?>"> - <?php p($rootCert->getCommonName()) ?> - </td> - <td title="<?php p($l->t('Valid until %s', $l->l('date', $rootCert->getExpireDate()))) ?>"> - <?php echo $l->l('date', $rootCert->getExpireDate()) ?> - </td> - <td title="<?php p($rootCert->getIssuerOrganization()) ?>"> - <?php p($rootCert->getIssuerName()) ?> - </td> - <td <?php if ($rootCert != ''): ?>class="remove" - <?php else: ?>style="visibility:hidden;" - <?php endif; ?>><img alt="<?php p($l->t('Delete')); ?>" - title="<?php p($l->t('Delete')); ?>" - class="action" - src="<?php print_unescaped(image_path('core', 'actions/delete.svg')); ?>"/> - </td> - </tr> - <?php endforeach; ?> - </tbody> - </table> - <form class="uploadButton" method="post" - action="<?php p($_['urlGenerator']->linkToRoute($_['uploadRoute'])); ?>" - target="certUploadFrame"> - <label for="rootcert_import" class="inlineblock button" - id="rootcert_import_button"><?php p($l->t('Import root certificate')); ?></label> - <input type="file" id="rootcert_import" name="rootcert_import" - class="hiddenuploadfield"> - </form> -</div> diff --git a/apps/settings/tests/AppInfo/ApplicationTest.php b/apps/settings/tests/AppInfo/ApplicationTest.php index 794adc25d7f..66994872a3d 100644 --- a/apps/settings/tests/AppInfo/ApplicationTest.php +++ b/apps/settings/tests/AppInfo/ApplicationTest.php @@ -25,12 +25,10 @@ namespace OCA\Settings\Tests\AppInfo; -use OC\User\Session; use OCA\Settings\AppInfo\Application; use OCA\Settings\Controller\AdminSettingsController; use OCA\Settings\Controller\AppSettingsController; use OCA\Settings\Controller\AuthSettingsController; -use OCA\Settings\Controller\CertificateController; use OCA\Settings\Controller\CheckSetupController; use OCA\Settings\Controller\LogSettingsController; use OCA\Settings\Controller\MailSettingsController; @@ -39,8 +37,6 @@ use OCA\Settings\Middleware\SubadminMiddleware; use OCP\AppFramework\Controller; use OCP\AppFramework\IAppContainer; use OCP\AppFramework\Middleware; -use OCP\IUser; -use OCP\IUserSession; use Test\TestCase; /** @@ -72,7 +68,6 @@ class ApplicationTest extends TestCase { [AdminSettingsController::class, Controller::class], [AppSettingsController::class, Controller::class], [AuthSettingsController::class, Controller::class], - // Needs session: [CertificateController::class, Controller::class], [CheckSetupController::class, Controller::class], [LogSettingsController::class, Controller::class], [MailSettingsController::class, Controller::class], @@ -90,31 +85,4 @@ class ApplicationTest extends TestCase { public function testContainerQuery($service, $expected) { $this->assertTrue($this->container->query($service) instanceof $expected); } - - public function dataContainerQueryRequiresSession() { - return [ - [CertificateController::class, Controller::class], - ]; - } - - /** - * @dataProvider dataContainerQueryRequiresSession - * @param string $service - * @param string $expected - */ - public function testContainerQueryRequiresSession($service, $expected) { - $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getUID') - ->willReturn('test'); - - $session = $this->createMock(IUserSession::class); - $session->expects($this->once()) - ->method('getUser') - ->willReturn($user); - - $this->overwriteService(Session::class, $session); - $this->assertTrue($this->container->query($service) instanceof $expected); - $this->restoreService(Session::class); - } } diff --git a/apps/settings/tests/Controller/CertificateControllerTest.php b/apps/settings/tests/Controller/CertificateControllerTest.php deleted file mode 100644 index 0259868321d..00000000000 --- a/apps/settings/tests/Controller/CertificateControllerTest.php +++ /dev/null @@ -1,196 +0,0 @@ -<?php -/** - * @copyright Copyright (c) 2015, ownCloud, Inc. - * - * @author Björn Schießle <bjoern@schiessle.org> - * @author Christoph Wurst <christoph@winzerhof-wurst.at> - * @author Joas Schilling <coding@schilljs.com> - * @author Lukas Reschke <lukas@statuscode.ch> - * @author Morris Jobke <hey@morrisjobke.de> - * @author Robin Appelman <robin@icewind.nl> - * @author Roeland Jago Douma <roeland@famdouma.nl> - * - * @license AGPL-3.0 - * - * This code is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License, version 3, - * as published by the Free Software Foundation. - * - * 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, version 3, - * along with this program. If not, see <http://www.gnu.org/licenses/> - * - */ - -namespace OCA\Settings\Tests\Controller; - -use OCA\Settings\Controller\CertificateController; -use OCP\App\IAppManager; -use OCP\AppFramework\Http; -use OCP\AppFramework\Http\DataResponse; -use OCP\ICertificateManager; -use OCP\IL10N; -use OCP\IRequest; - -/** - * Class CertificateControllerTest - * - * @package Tests\Settings\Controller - */ -class CertificateControllerTest extends \Test\TestCase { - /** @var CertificateController */ - private $certificateController; - /** @var IRequest */ - private $request; - /** @var ICertificateManager */ - private $certificateManager; - /** @var IL10N */ - private $l10n; - /** @var IAppManager */ - private $appManager; - /** @var ICertificateManager */ - private $systemCertificateManager; - - protected function setUp(): void { - parent::setUp(); - - $this->request = $this->getMockBuilder(IRequest::class)->getMock(); - $this->certificateManager = $this->getMockBuilder(ICertificateManager::class)->getMock(); - $this->systemCertificateManager = $this->getMockBuilder(ICertificateManager::class)->getMock(); - $this->l10n = $this->getMockBuilder(IL10N::class)->getMock(); - $this->appManager = $this->getMockBuilder(IAppManager::class)->getMock(); - - $this->certificateController = $this->getMockBuilder(CertificateController::class) - ->setConstructorArgs( - [ - 'settings', - $this->request, - $this->certificateManager, - $this->systemCertificateManager, - $this->l10n, - $this->appManager - ] - )->setMethods(['isCertificateImportAllowed'])->getMock(); - - $this->certificateController->expects($this->any()) - ->method('isCertificateImportAllowed')->willReturn(true); - } - - public function testAddPersonalRootCertificateWithEmptyFile() { - $this->request - ->expects($this->once()) - ->method('getUploadedFile') - ->with('rootcert_import') - ->willReturn(null); - - $expected = new DataResponse(['message' => 'No file uploaded'], Http::STATUS_UNPROCESSABLE_ENTITY); - $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); - } - - public function testAddPersonalRootCertificateValidCertificate() { - $uploadedFile = [ - 'tmp_name' => __DIR__ . '/../../../../tests/data/certificates/goodCertificate.crt', - 'name' => 'goodCertificate.crt', - ]; - - $certificate = $this->getMockBuilder('\OCP\ICertificate')->getMock(); - $certificate - ->expects($this->once()) - ->method('getName') - ->willReturn('Name'); - $certificate - ->expects($this->once()) - ->method('getCommonName') - ->willReturn('CommonName'); - $certificate - ->expects($this->once()) - ->method('getOrganization') - ->willReturn('Organization'); - $certificate - ->expects($this->exactly(2)) - ->method('getIssueDate') - ->willReturn(new \DateTime('@1429099555')); - $certificate - ->expects($this->exactly(2)) - ->method('getExpireDate') - ->willReturn(new \DateTime('@1529099555')); - $certificate - ->expects($this->once()) - ->method('getIssuerName') - ->willReturn('Issuer'); - $certificate - ->expects($this->once()) - ->method('getIssuerOrganization') - ->willReturn('IssuerOrganization'); - - $this->request - ->expects($this->once()) - ->method('getUploadedFile') - ->with('rootcert_import') - ->willReturn($uploadedFile); - $this->certificateManager - ->expects($this->once()) - ->method('addCertificate') - ->with(file_get_contents($uploadedFile['tmp_name'], 'goodCertificate.crt')) - ->willReturn($certificate); - - $this->l10n - ->expects($this->at(0)) - ->method('l') - ->with('date', new \DateTime('@1429099555')) - ->willReturn('Valid From as String'); - $this->l10n - ->expects($this->at(1)) - ->method('l') - ->with('date', new \DateTime('@1529099555')) - ->willReturn('Valid Till as String'); - - - $expected = new DataResponse([ - 'name' => 'Name', - 'commonName' => 'CommonName', - 'organization' => 'Organization', - 'validFrom' => 1429099555, - 'validTill' => 1529099555, - 'validFromString' => 'Valid From as String', - 'validTillString' => 'Valid Till as String', - 'issuer' => 'Issuer', - 'issuerOrganization' => 'IssuerOrganization', - ]); - $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); - } - - public function testAddPersonalRootCertificateInvalidCertificate() { - $uploadedFile = [ - 'tmp_name' => __DIR__ . '/../../../../tests/data/certificates/badCertificate.crt', - 'name' => 'badCertificate.crt', - ]; - - $this->request - ->expects($this->once()) - ->method('getUploadedFile') - ->with('rootcert_import') - ->willReturn($uploadedFile); - $this->certificateManager - ->expects($this->once()) - ->method('addCertificate') - ->with(file_get_contents($uploadedFile['tmp_name'], 'badCertificate.crt')) - ->will($this->throwException(new \Exception())); - - $expected = new DataResponse(['An error occurred.'], Http::STATUS_UNPROCESSABLE_ENTITY); - $this->assertEquals($expected, $this->certificateController->addPersonalRootCertificate()); - } - - public function testRemoveCertificate() { - $this->certificateManager - ->expects($this->once()) - ->method('removeCertificate') - ->with('CertificateToRemove'); - - $this->assertEquals(new DataResponse(), $this->certificateController->removePersonalRootCertificate('CertificateToRemove')); - } -} |