From 4a319ef21ae5527a6ac4bd309d90139084d0d56f Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jul 2020 09:22:32 +0200 Subject: [PATCH 1/3] Remove unnecessary compatibility layers Signed-off-by: Joas Schilling --- lib/AppInfo/Application.php | 14 ++-- lib/Controller/ApiController.php | 10 +-- lib/Db/RegistrationMapper.php | 5 +- lib/Util/CoreBridge.php | 78 --------------------- tests/Unit/Controller/ApiControllerTest.php | 11 +-- 5 files changed, 21 insertions(+), 97 deletions(-) delete mode 100644 lib/Util/CoreBridge.php diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index c5c9309..fd82a0d 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -23,19 +23,19 @@ namespace OCA\Registration\AppInfo; +use OCA\Registration\Capabilities; use OCP\AppFramework\App; +use OC\Authentication\Token\IProvider; class Application extends App { - public function __construct(array $urlParams = []) { - parent::__construct('registration', $urlParams); + public function __construct() { + parent::__construct('registration'); $container = $this->getContainer(); - $container->registerService('OC\Authentication\Token\IProvider', function ($c) { - return \OC::$server->query('OC\Authentication\Token\IProvider'); + $container->registerService(IProvider::class, function ($c) { + return \OC::$server->query(IProvider::class); // TODO needed? }); - if (interface_exists('\OCP\Capabilities\IPublicCapability')) { - $container->registerCapability(\OCA\Registration\Capabilities::class); - } + $container->registerCapability(Capabilities::class); } } diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index c7847e9..f217525 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -27,9 +27,11 @@ use OCA\Registration\Db\Registration; use OCA\Registration\Service\MailService; use OCA\Registration\Service\RegistrationException; use OCA\Registration\Service\RegistrationService; -use OCA\Registration\Util\CoreBridge; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; +use OCP\AppFramework\OCS\OCSBadRequestException; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\AppFramework\Http\DataResponse; use OCP\Defaults; @@ -80,7 +82,7 @@ class ApiController extends OCSController { $this->registrationService->validateDisplayname($displayname); $this->registrationService->validateUsername($username); } catch (RegistrationException $e) { - throw CoreBridge::createException('OCSBadRequestException', $e->getMessage()); + throw new OCSBadRequestException($e->getMessage()); } $data = [ 'username' => $username, @@ -103,7 +105,7 @@ class ApiController extends OCSController { /** @var Registration $registration */ $registration = $this->registrationService->getRegistrationForSecret($clientSecret); } catch (DoesNotExistException $e) { - throw CoreBridge::createException('OCSNotFoundException', 'No pending registration.'); + throw new OCSNotFoundException('No pending registration.'); } if (!$registration->getEmailConfirmed()) { @@ -173,7 +175,7 @@ class ApiController extends OCSController { } return new DataResponse($data, Http::STATUS_OK); } catch (RegistrationException $exception) { - throw CoreBridge::createException('OCSException', $exception->getMessage(), $exception->getCode()); + throw new OCSException($exception->getMessage(), $exception->getCode()); } } } diff --git a/lib/Db/RegistrationMapper.php b/lib/Db/RegistrationMapper.php index 56d2a75..6c144a3 100644 --- a/lib/Db/RegistrationMapper.php +++ b/lib/Db/RegistrationMapper.php @@ -111,7 +111,7 @@ class RegistrationMapper extends QBMapper { * @param Registration $registration */ public function generateNewToken(Registration $registration): void { - $token = $this->random->generate(10, ISecureRandom::CHAR_UPPER.ISecureRandom::CHAR_LOWER.ISecureRandom::CHAR_DIGITS); + $token = $this->random->generate(10, ISecureRandom::CHAR_HUMAN_READABLE); $registration->setToken($token); } @@ -119,8 +119,7 @@ class RegistrationMapper extends QBMapper { * @param Registration $registration */ public function generateClientSecret(Registration $registration): void { - $token = $this->random->generate(32, 'abcdefgijkmnopqrstwxyzABCDEFGHJKLMNPQRSTWXYZ23456789'); - //FIXME eqivalent to ISecureRandom::CHAR_HUMAN_READABLE introduced in https://github.com/nextcloud/server/commit/f2a2b34e4639e88f8d948a388a51f010212b42a3 but not supported in ownCloud yet. We'll just use the string for now then switch to constants when supported. + $token = $this->random->generate(32, ISecureRandom::CHAR_HUMAN_READABLE); $registration->setClientSecret($token); } } diff --git a/lib/Util/CoreBridge.php b/lib/Util/CoreBridge.php deleted file mode 100644 index 3875dd5..0000000 --- a/lib/Util/CoreBridge.php +++ /dev/null @@ -1,78 +0,0 @@ - [ - 'OCP\AppFramework\OCS\OCSException', - 'OC\OCS\Exception', - ], - 'OCSBadRequestException' => [ - 'OCP\AppFramework\OCS\OCSBadRequestException', - 'OC\OCS\Exception', - ], - 'OCSNotFoundException' => [ - 'OCP\AppFramework\OCS\OCSNotFoundException', - 'OC\OCS\Exception', - ], - 'DoesNotExistException' => [ - 'OCP\AppFramework\Db\DoesNotExistException', - 'OCP\AppFramework\Db\DoesNotExistException', - ], - ]; - - if (!array_key_exists($className, $classes)) { - throw new \LogicException('No valid exception class found'); - } - - foreach ($classes[$className] as $class) { - if (class_exists($class)) { - return $class; - } - } - - throw new \LogicException('No valid exception class found'); - } - - /** - * @param string $className - * @param null|string $message - * @param null|int $code - * @return \Exception - */ - public static function createException($className, $message = null, $code = null) { - $exceptionClassName = self::exceptionClass($className); - - $reflection = new \ReflectionClass($exceptionClassName); - $params = $reflection->getConstructor()->getParameters(); - - if ($params[0]->getClass() && ($params[0]->getClass()->getName() === 'OC\OCS\Result' || $params[0]->getClass()->getName() === 'OC_OCS_Result')) { - $subClass = $params[0]->getClass()->getName(); - return new $exceptionClassName(new $subClass($message, $code)); - } - - if (count($params) >= 2) { - if ($params[1]->getClass() && $params[1]->getClass()->getName() === 'Exception') { - return new $exceptionClassName($message); - } - - return new $exceptionClassName($message, $code); - } - - if ($exceptionClassName === 'OCP\AppFramework\OCS\OCSNotFoundException') { - return new $exceptionClassName($message); - } - - return new $exceptionClassName(); - } -} diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index f4b3351..2701f42 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -15,9 +15,10 @@ use OCA\Registration\Controller\ApiController; use OCA\Registration\Db\Registration; use OCA\Registration\Service\MailService; use OCA\Registration\Service\RegistrationService; -use OCA\Registration\Util\CoreBridge; use OCP\AppFramework\Http; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\OCS\OCSException; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\Defaults; use OCP\IL10N; use OCP\IRequest; @@ -80,7 +81,7 @@ class ApiControllerTest extends TestCase { } public function testValidateFailEmail() { - $exception = CoreBridge::createException('OCSException', '', 999); + $exception = new OCSException('', 999); $this->expectException(get_class($exception)); @@ -93,7 +94,7 @@ class ApiControllerTest extends TestCase { } public function testValidateFailDisplayname() { - $exception = CoreBridge::createException('OCSException', '', 999); + $exception = new OCSException('', 999); $this->expectException(get_class($exception)); @@ -106,7 +107,7 @@ class ApiControllerTest extends TestCase { } public function testValidateFailUsername() { - $exception = CoreBridge::createException('OCSException', '', 999); + $exception = new OCSException('', 999); $this->expectException(get_class($exception)); @@ -119,7 +120,7 @@ class ApiControllerTest extends TestCase { } public function testStatusNoRegistration() { - $exception = CoreBridge::createException('OCSNotFoundException', '', 404); + $exception = new OCSNotFoundException(''); $this->expectException(get_class($exception)); From 7d1fd1b49a8dc60715abd91c833ba41b6307fa16 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jul 2020 11:31:27 +0200 Subject: [PATCH 2/3] Simplify the settings module Signed-off-by: Joas Schilling --- lib/Controller/SettingsController.php | 43 ++------------ lib/Settings/RegistrationSettings.php | 83 +++++++++++++++++++++------ 2 files changed, 71 insertions(+), 55 deletions(-) diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index a487cf5..4c809e3 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -13,7 +13,6 @@ namespace OCA\Registration\Controller; use \OCP\IRequest; -use \OCP\AppFramework\Http\TemplateResponse; use \OCP\AppFramework\Http\DataResponse; use \OCP\AppFramework\Http; use \OCP\AppFramework\Controller; @@ -40,8 +39,6 @@ class SettingsController extends Controller { $this->appName = $appName; } - - /** * @AdminRequired * @@ -71,53 +68,25 @@ class SettingsController extends Controller { $this->config->deleteAppValue($this->appName, 'registered_user_group'); return new DataResponse([ 'data' => [ - 'message' => (string) $this->l10n->t('Saved'), + 'message' => $this->l10n->t('Saved'), ], - 'status' => 'success' - + 'status' => 'success', ]); } elseif (in_array($registered_user_group, $group_id_list)) { $this->config->setAppValue($this->appName, 'registered_user_group', $registered_user_group); return new DataResponse([ 'data' => [ - 'message' => (string) $this->l10n->t('Saved'), + 'message' => $this->l10n->t('Saved'), ], - 'status' => 'success' + 'status' => 'success', ]); } else { return new DataResponse([ 'data' => [ - 'message' => (string) $this->l10n->t('No such group'), + 'message' => $this->l10n->t('No such group'), ], - 'status' => 'error' + 'status' => 'error', ], Http::STATUS_NOT_FOUND); } } - /** - * @AdminRequired - * - * @return TemplateResponse - */ - public function displayPanel() { - // handle groups - $groups = $this->groupmanager->search(''); - $group_id_list = []; - foreach ($groups as $group) { - $group_id_list[] = $group->getGid(); - } - $current_value = $this->config->getAppValue($this->appName, 'registered_user_group', 'none'); - - // handle domains - $allowed_domains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); - - // handle admin validation - $admin_approval_required = $this->config->getAppValue($this->appName, 'admin_approval_required', "no"); - - return new TemplateResponse('registration', 'admin', [ - 'groups' => $group_id_list, - 'current' => $current_value, - 'allowed' => $allowed_domains, - 'approval_required' => $admin_approval_required - ], ''); - } } diff --git a/lib/Settings/RegistrationSettings.php b/lib/Settings/RegistrationSettings.php index bbb36fb..0f9ed51 100644 --- a/lib/Settings/RegistrationSettings.php +++ b/lib/Settings/RegistrationSettings.php @@ -1,33 +1,80 @@ + * + * @author Pellaeon Lin + * + * @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 . + * + */ + namespace OCA\Registration\Settings; +use OCP\AppFramework\Http\TemplateResponse; +use OCP\IConfig; +use OCP\IGroupManager; use OCP\Settings\ISettings; -use OCA\Registration\Controller\SettingsController; - class RegistrationSettings implements ISettings { - public function getForm() { - $controller = \OC::$server->query(SettingsController::class); - return $controller->displayPanel(); + /** @var IConfig */ + private $config; + /** @var IGroupManager */ + private $groupManager; + /** @var string */ + protected $appName; + + public function __construct(string $appName, + IConfig $config, + IGroupManager $groupManager) { + $this->appName = $appName; + $this->config = $config; + $this->groupManager = $groupManager; + $this->appName = $appName; } - public function getSection() { + public function getForm(): TemplateResponse { + // handle groups + $groups = $this->groupManager->search(''); + $groupIds = []; + foreach ($groups as $group) { + $groupIds[] = $group->getGid(); + } + $assignedGroups = $this->config->getAppValue($this->appName, 'registered_user_group', 'none'); + + // handle domains + $allowedDomains = $this->config->getAppValue($this->appName, 'allowed_domains', ''); + + // handle admin validation + $adminApprovalRequired = $this->config->getAppValue($this->appName, 'admin_approval_required', "no"); + + return new TemplateResponse('registration', 'admin', [ + 'groups' => $groupIds, + 'current' => $assignedGroups, + 'allowed' => $allowedDomains, + 'approval_required' => $adminApprovalRequired + ], ''); + } + + public function getSection(): string { return 'additional'; } - public function getPriority() { + public function getPriority(): int { return 50; } - - /* - * Below for ownCloud - */ - public function getPanel() { - return $this->getForm(); - } - - public function getSectionID() { - return $this->getSection(); - } } From 528b0549dcd1abff0daf17068820181a6eee4426 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jul 2020 16:50:14 +0200 Subject: [PATCH 3/3] On supported versions the version is always > 12 Signed-off-by: Joas Schilling --- templates/form.php | 4 +--- templates/message.php | 4 +--- templates/register.php | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/templates/form.php b/templates/form.php index 5253ec9..ea426a0 100644 --- a/templates/form.php +++ b/templates/form.php @@ -1,9 +1,7 @@ = 12) { - \OCP\Util::addStyle('core', 'guest'); -} +\OCP\Util::addStyle('core', 'guest'); ?>
diff --git a/templates/message.php b/templates/message.php index 7c8a315..b583958 100644 --- a/templates/message.php +++ b/templates/message.php @@ -1,8 +1,6 @@ = 12) { - \OCP\Util::addStyle('core', 'guest'); -} +\OCP\Util::addStyle('core', 'guest'); ?>
    • diff --git a/templates/register.php b/templates/register.php index cccd8ba..3cfc6d6 100644 --- a/templates/register.php +++ b/templates/register.php @@ -1,8 +1,6 @@ = 12) { - \OCP\Util::addStyle('core', 'guest'); -} +\OCP\Util::addStyle('core', 'guest'); if ($_['entered']): ?>