From 391a368557b18a89c79406d366e578530e36fe36 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 29 Jan 2020 10:42:11 +0100 Subject: [PATCH] Move the personal 2FA settings to its own app Small isolated classes are easier to understand and to maintain! Another step in the cleaning up of the settings. So that adding new stuff later is easier. Signed-off-by: Roeland Jago Douma --- apps/settings/appinfo/info.xml | 1 + .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Settings/Personal/Security.php | 54 ---------- .../Settings/Personal/Security/TwoFactor.php | 98 +++++++++++++++++++ .../templates/settings/personal/security.php | 45 --------- .../settings/personal/security/twofactor.php | 70 +++++++++++++ .../tests/Settings/Personal/SecurityTest.php | 35 ------- 8 files changed, 171 insertions(+), 134 deletions(-) create mode 100644 apps/settings/lib/Settings/Personal/Security/TwoFactor.php create mode 100644 apps/settings/templates/settings/personal/security/twofactor.php diff --git a/apps/settings/appinfo/info.xml b/apps/settings/appinfo/info.xml index 61bfe454ca..d86e5395ef 100644 --- a/apps/settings/appinfo/info.xml +++ b/apps/settings/appinfo/info.xml @@ -24,6 +24,7 @@ OCA\Settings\Sections\Admin\Security OCA\Settings\Sections\Admin\Server OCA\Settings\Sections\Admin\Sharing + OCA\Settings\Settings\Personal\Security\TwoFactor OCA\Settings\Sections\Personal\PersonalInfo OCA\Settings\Sections\Personal\Security OCA\Settings\Sections\Personal\SyncClients diff --git a/apps/settings/composer/composer/autoload_classmap.php b/apps/settings/composer/composer/autoload_classmap.php index 52c4f4de47..cf7963ef90 100644 --- a/apps/settings/composer/composer/autoload_classmap.php +++ b/apps/settings/composer/composer/autoload_classmap.php @@ -50,4 +50,5 @@ return array( 'OCA\\Settings\\Sections\\Personal\\PersonalInfo' => $baseDir . '/../lib/Sections/Personal/PersonalInfo.php', 'OCA\\Settings\\Sections\\Personal\\Security' => $baseDir . '/../lib/Sections/Personal/Security.php', 'OCA\\Settings\\Sections\\Personal\\SyncClients' => $baseDir . '/../lib/Sections/Personal/SyncClients.php', + 'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => $baseDir . '/../lib/Settings/Personal/Security/TwoFactor.php', ); diff --git a/apps/settings/composer/composer/autoload_static.php b/apps/settings/composer/composer/autoload_static.php index 3c81d42c97..f9805dcff1 100644 --- a/apps/settings/composer/composer/autoload_static.php +++ b/apps/settings/composer/composer/autoload_static.php @@ -65,6 +65,7 @@ class ComposerStaticInitSettings 'OCA\\Settings\\Sections\\Personal\\PersonalInfo' => __DIR__ . '/..' . '/../lib/Sections/Personal/PersonalInfo.php', 'OCA\\Settings\\Sections\\Personal\\Security' => __DIR__ . '/..' . '/../lib/Sections/Personal/Security.php', 'OCA\\Settings\\Sections\\Personal\\SyncClients' => __DIR__ . '/..' . '/../lib/Sections/Personal/SyncClients.php', + 'OCA\\Settings\\Settings\\Personal\\Security\\TwoFactor' => __DIR__ . '/..' . '/../lib/Settings/Personal/Security/TwoFactor.php', ); public static function getInitializer(ClassLoader $loader) diff --git a/apps/settings/lib/Settings/Personal/Security.php b/apps/settings/lib/Settings/Personal/Security.php index 192b082d6b..f571be0c22 100644 --- a/apps/settings/lib/Settings/Personal/Security.php +++ b/apps/settings/lib/Settings/Personal/Security.php @@ -27,25 +27,8 @@ namespace OCA\Settings\Personal; - -use function array_filter; -use function array_map; -use function is_null; -use OC\Authentication\Exceptions\InvalidTokenException; -use OC\Authentication\Token\INamedToken; -use OC\Authentication\Token\IProvider as IAuthTokenProvider; -use OC\Authentication\Token\IToken; -use OC\Authentication\TwoFactorAuth\Manager as TwoFactorManager; -use OC\Authentication\TwoFactorAuth\ProviderLoader; use OCP\AppFramework\Http\TemplateResponse; -use OCP\Authentication\TwoFactorAuth\IProvider; -use OCP\Authentication\TwoFactorAuth\IProvidesPersonalSettings; -use OCP\IConfig; -use OCP\IInitialStateService; -use OCP\ISession; use OCP\IUserManager; -use OCP\IUserSession; -use OCP\Session\Exceptions\SessionNotAvailableException; use OCP\Settings\ISettings; class Security implements ISettings { @@ -53,28 +36,13 @@ class Security implements ISettings { /** @var IUserManager */ private $userManager; - /** @var ProviderLoader */ - private $providerLoader; - - /** @var IUserSession */ - private $userSession; - /** @var string|null */ private $uid; - /** @var IConfig */ - private $config; - public function __construct(IUserManager $userManager, - ProviderLoader $providerLoader, - IUserSession $userSession, - IConfig $config, ?string $UserId) { $this->userManager = $userManager; - $this->providerLoader = $providerLoader; - $this->userSession = $userSession; $this->uid = $UserId; - $this->config = $config; } public function getForm(): TemplateResponse { @@ -86,10 +54,7 @@ class Security implements ISettings { return new TemplateResponse('settings', 'settings/personal/security', [ 'passwordChangeSupported' => $passwordChangeSupported, - 'twoFactorProviderData' => $this->getTwoFactorProviderData(), - 'themedark' => $this->config->getUserValue($this->uid, 'accessibility', 'theme', false) ]); - } public function getSection(): string { @@ -99,23 +64,4 @@ class Security implements ISettings { public function getPriority(): int { return 10; } - - private function getTwoFactorProviderData(): array { - $user = $this->userSession->getUser(); - if (is_null($user)) { - // Actually impossible, but still … - return []; - } - - return [ - 'providers' => array_map(function (IProvidesPersonalSettings $provider) use ($user) { - return [ - 'provider' => $provider, - 'settings' => $provider->getPersonalSettings($user) - ]; - }, array_filter($this->providerLoader->getProviders($user), function (IProvider $provider) { - return $provider instanceof IProvidesPersonalSettings; - })) - ]; - } } diff --git a/apps/settings/lib/Settings/Personal/Security/TwoFactor.php b/apps/settings/lib/Settings/Personal/Security/TwoFactor.php new file mode 100644 index 0000000000..b51e4942b8 --- /dev/null +++ b/apps/settings/lib/Settings/Personal/Security/TwoFactor.php @@ -0,0 +1,98 @@ + + * + * @author Christoph Wurst + * @author Roeland Jago Douma + * + * @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\Settings\Settings\Personal\Security; + +use function array_filter; +use function array_map; +use function is_null; +use OC\Authentication\TwoFactorAuth\ProviderLoader; +use OCP\AppFramework\Http\TemplateResponse; +use OCP\Authentication\TwoFactorAuth\IProvider; +use OCP\Authentication\TwoFactorAuth\IProvidesPersonalSettings; +use OCP\IConfig; +use OCP\IUserSession; +use OCP\Settings\ISettings; + +class TwoFactor implements ISettings { + + /** @var ProviderLoader */ + private $providerLoader; + + /** @var IUserSession */ + private $userSession; + + /** @var string|null */ + private $uid; + + /** @var IConfig */ + private $config; + + public function __construct(ProviderLoader $providerLoader, + IUserSession $userSession, + IConfig $config, + ?string $UserId) { + $this->providerLoader = $providerLoader; + $this->userSession = $userSession; + $this->uid = $UserId; + $this->config = $config; + } + + public function getForm(): TemplateResponse { + return new TemplateResponse('settings', 'settings/personal/security/twofactor', [ + 'twoFactorProviderData' => $this->getTwoFactorProviderData(), + 'themedark' => $this->config->getUserValue($this->uid, 'accessibility', 'theme', false) + ]); + + } + + public function getSection(): string { + return 'security'; + } + + public function getPriority(): int { + return 15; + } + + private function getTwoFactorProviderData(): array { + $user = $this->userSession->getUser(); + if (is_null($user)) { + // Actually impossible, but still … + return []; + } + + return [ + 'providers' => array_map(function (IProvidesPersonalSettings $provider) use ($user) { + return [ + 'provider' => $provider, + 'settings' => $provider->getPersonalSettings($user) + ]; + }, array_filter($this->providerLoader->getProviders($user), function (IProvider $provider) { + return $provider instanceof IProvidesPersonalSettings; + })) + ]; + } +} diff --git a/apps/settings/templates/settings/personal/security.php b/apps/settings/templates/settings/personal/security.php index e72b443f59..23959f7d9c 100644 --- a/apps/settings/templates/settings/personal/security.php +++ b/apps/settings/templates/settings/personal/security.php @@ -59,48 +59,3 @@ if($_['passwordChangeSupported']) { - -
-

t('Two-Factor Authentication'));?>

- -

t('Use a second factor besides your password to increase security for your account.'));?>

-
    - -
  • - getLightIcon(); - } - else { - $icon = $provider->getDarkIcon(); - } - //fallback icon if the 2factor provider doesn't provide an icon. - } else { - if ($_['themedark']) { - $icon = image_path('core', 'actions/password-white.svg'); - } - else { - $icon = image_path('core', 'actions/password.svg'); - } - - } - /** @var \OCP\Authentication\TwoFactorAuth\IPersonalProviderSettings $settings */ - $settings = $data['settings']; - ?> -

    - - getDisplayName()) ?> -

    - getBody()->fetchPage()) ?> -
    • - -
-
- diff --git a/apps/settings/templates/settings/personal/security/twofactor.php b/apps/settings/templates/settings/personal/security/twofactor.php new file mode 100644 index 0000000000..f5e5b7025c --- /dev/null +++ b/apps/settings/templates/settings/personal/security/twofactor.php @@ -0,0 +1,70 @@ + + * + * @author Roeland Jago Douma + * + * @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 . + * + */ + +?> + +
+

t('Two-Factor Authentication'));?>

+ +

t('Use a second factor besides your password to increase security for your account.'));?>

+
    + +
  • + getLightIcon(); + } + else { + $icon = $provider->getDarkIcon(); + } + //fallback icon if the 2factor provider doesn't provide an icon. + } else { + if ($_['themedark']) { + $icon = image_path('core', 'actions/password-white.svg'); + } + else { + $icon = image_path('core', 'actions/password.svg'); + } + + } + /** @var \OCP\Authentication\TwoFactorAuth\IPersonalProviderSettings $settings */ + $settings = $data['settings']; + ?> +

    + + getDisplayName()) ?> +

    + getBody()->fetchPage()) ?> +
    • + +
+
+ diff --git a/apps/settings/tests/Settings/Personal/SecurityTest.php b/apps/settings/tests/Settings/Personal/SecurityTest.php index bd087afa46..103bd9dae9 100644 --- a/apps/settings/tests/Settings/Personal/SecurityTest.php +++ b/apps/settings/tests/Settings/Personal/SecurityTest.php @@ -43,15 +43,6 @@ class SecurityTest extends TestCase { /** @var IUserManager|MockObject */ private $userManager; - /** @var ProviderLoader|MockObject */ - private $providerLoader; - - /** @var IUserSession|MockObject */ - private $userSession; - - /** @var IConfig|MockObject */ - private $config; - /** @var string */ private $uid; @@ -62,16 +53,10 @@ class SecurityTest extends TestCase { parent::setUp(); $this->userManager = $this->createMock(IUserManager::class); - $this->providerLoader = $this->createMock(ProviderLoader::class); - $this->userSession = $this->createMock(IUserSession::class); - $this->config = $this->createMock(IConfig::class); $this->uid = 'test123'; $this->section = new Security( $this->userManager, - $this->providerLoader, - $this->userSession, - $this->config, $this->uid ); } @@ -85,31 +70,11 @@ class SecurityTest extends TestCase { $user->expects($this->once()) ->method('canChangePassword') ->willReturn(true); - $this->userSession->expects($this->once()) - ->method('getUser') - ->willReturn($user); - $this->providerLoader->expects($this->once()) - ->method('getProviders') - ->with($user) - ->willReturn([]); - $this->config->expects($this->once()) - ->method('getUserValue') - ->with( - $this->uid, - 'accessibility', - 'theme', - false - ) - ->willReturn(false); $form = $this->section->getForm(); $expected = new TemplateResponse('settings', 'settings/personal/security', [ 'passwordChangeSupported' => true, - 'twoFactorProviderData' => [ - 'providers' => [], - ], - 'themedark' => false, ]); $this->assertEquals($expected, $form); }