Merge pull request #19192 from nextcloud/enh/settings/move_2fa_to_own_settings_class

Move the personal 2FA settings to its settings

apps/settings/appinfo/info.xml

@@ -24,6 +24,7 @@
 		<admin-section>OCA\Settings\Sections\Admin\Security</admin-section>
 		<admin-section>OCA\Settings\Sections\Admin\Server</admin-section>
 		<admin-section>OCA\Settings\Sections\Admin\Sharing</admin-section>
+		<personal>OCA\Settings\Settings\Personal\Security\TwoFactor</personal>
 		<personal-section>OCA\Settings\Sections\Personal\PersonalInfo</personal-section>
 		<personal-section>OCA\Settings\Sections\Personal\Security</personal-section>
 		<personal-section>OCA\Settings\Sections\Personal\SyncClients</personal-section>

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

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)

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

apps/settings/lib/Settings/Personal/Security/TwoFactor.php

@@ -0,0 +1,98 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * @copyright Copyright (c) 2019, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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;
+			}))
+		];
+	}
+}

apps/settings/templates/settings/personal/security.php

@@ -59,48 +59,3 @@ if($_['passwordChangeSupported']) {
 	<span class="msg"></span>
 </div>
 <?php } ?>
-
-<div id="two-factor-auth" class="section">
-	<h2><?php p($l->t('Two-Factor Authentication'));?></h2>
-	<a target="_blank" rel="noreferrer noopener" class="icon-info"
-	   title="<?php p($l->t('Open documentation'));?>"
-	   href="<?php p(link_to_docs('user-2fa')); ?>"></a>
-	<p class="settings-hint"><?php p($l->t('Use a second factor besides your password to increase security for your account.'));?></p>
-	<ul>
-	<?php foreach ($_['twoFactorProviderData']['providers'] as $data) { ?>
-		<li>
-			<?php
-
-			/** @var \OCP\Authentication\TwoFactorAuth\IProvidesPersonalSettings $provider */
-			$provider = $data['provider'];
-			//Handle 2FA provider icons and theme
-			if ($provider instanceof \OCP\Authentication\TwoFactorAuth\IProvidesIcons) {
-				if ($_['themedark']) {
-					$icon = $provider->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'];
-			?>
-			<h3>
-				<img class="two-factor-provider-settings-icon" src="<?php p($icon) ?>" alt="">
-				<?php p($provider->getDisplayName()) ?>
-			</h3>
-			<?php print_unescaped($settings->getBody()->fetchPage()) ?>
-		</li>
-	<?php } ?>
-	</ul>
-</div>
-

apps/settings/templates/settings/personal/security/twofactor.php

@@ -0,0 +1,70 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2020, Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @author Roeland Jago Douma <roeland@famdouma.nl>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+?>
+
+<div id="two-factor-auth" class="section">
+	<h2><?php p($l->t('Two-Factor Authentication'));?></h2>
+	<a target="_blank" rel="noreferrer noopener" class="icon-info"
+	   title="<?php p($l->t('Open documentation'));?>"
+	   href="<?php p(link_to_docs('user-2fa')); ?>"></a>
+	<p class="settings-hint"><?php p($l->t('Use a second factor besides your password to increase security for your account.'));?></p>
+	<ul>
+	<?php foreach ($_['twoFactorProviderData']['providers'] as $data) { ?>
+		<li>
+			<?php
+
+			/** @var \OCP\Authentication\TwoFactorAuth\IProvidesPersonalSettings $provider */
+			$provider = $data['provider'];
+			//Handle 2FA provider icons and theme
+			if ($provider instanceof \OCP\Authentication\TwoFactorAuth\IProvidesIcons) {
+				if ($_['themedark']) {
+					$icon = $provider->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'];
+			?>
+			<h3>
+				<img class="two-factor-provider-settings-icon" src="<?php p($icon) ?>" alt="">
+				<?php p($provider->getDisplayName()) ?>
+			</h3>
+			<?php print_unescaped($settings->getBody()->fetchPage()) ?>
+		</li>
+	<?php } ?>
+	</ul>
+</div>
+

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