Merge pull request #21224 from nextcloud/techdebt/noid/remove-unreachable-controller-and-route

Remove controller and routes which have no UI component anyway

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

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

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

apps/settings/lib/Controller/CertificateController.php

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

apps/settings/templates/certificates.php

@@ -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>

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

apps/settings/tests/Controller/CertificateControllerTest.php

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