Split personal security settings in code

Instead of one big monolitic sections this is the first step in breaking
down the settings. This should make is easiet to see what does what. As
well as nicely splitting up the sections.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

lib/composer/composer/autoload_classmap.php

@@ -1167,6 +1167,7 @@ return array(
     'OC\\Settings\\Personal\\Additional' => $baseDir . '/settings/Settings/Personal/Additional.php',
     'OC\\Settings\\Personal\\PersonalInfo' => $baseDir . '/settings/Settings/Personal/PersonalInfo.php',
     'OC\\Settings\\Personal\\Security' => $baseDir . '/settings/Settings/Personal/Security.php',
+    'OC\\Settings\\Personal\\Security\\Authtokens' => $baseDir . '/settings/Settings/Personal/Security/Authtokens.php',
     'OC\\Settings\\Personal\\ServerDevNotice' => $baseDir . '/settings/Settings/Personal/ServerDevNotice.php',
     'OC\\Settings\\Section' => $baseDir . '/lib/private/Settings/Section.php',
     'OC\\Setup' => $baseDir . '/lib/private/Setup.php',

lib/composer/composer/autoload_static.php

@@ -1201,6 +1201,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
         'OC\\Settings\\Personal\\Additional' => __DIR__ . '/../../..' . '/settings/Settings/Personal/Additional.php',
         'OC\\Settings\\Personal\\PersonalInfo' => __DIR__ . '/../../..' . '/settings/Settings/Personal/PersonalInfo.php',
         'OC\\Settings\\Personal\\Security' => __DIR__ . '/../../..' . '/settings/Settings/Personal/Security.php',
+        'OC\\Settings\\Personal\\Security\\Authtokens' => __DIR__ . '/../../..' . '/settings/Settings/Personal/Security/Authtokens.php',
         'OC\\Settings\\Personal\\ServerDevNotice' => __DIR__ . '/../../..' . '/settings/Settings/Personal/ServerDevNotice.php',
         'OC\\Settings\\Section' => __DIR__ . '/../../..' . '/lib/private/Settings/Section.php',
         'OC\\Setup' => __DIR__ . '/../../..' . '/lib/private/Setup.php',

lib/private/Settings/Manager.php

@@ -298,6 +298,10 @@ class Manager implements IManager {
 			/** @var ISettings $form */
 			$form = $this->container->query(Personal\Security::class);
 			$forms[$form->getPriority()] = [$form];
+
+			/** @var ISettings $form */
+			$form = $this->container->query(Personal\Security\Authtokens::class);
+			$forms[$form->getPriority()] = [$form];
 		}
 		if ($section === 'additional') {
 			/** @var ISettings $form */

settings/Settings/Personal/Security.php

@@ -49,69 +49,37 @@ class Security implements ISettings {
 	/** @var IUserManager */
 	private $userManager;
 
-	/** @var TwoFactorManager */
-	private $twoFactorManager;
-
-	/** @var IAuthTokenProvider */
-	private $tokenProvider;
-
 	/** @var ProviderLoader */
 	private $providerLoader;
 
 	/** @var IUserSession */
 	private $userSession;
 
-	/** @var ISession */
-	private $session;
-
-	/** @var IInitialStateService */
-	private $initialStateService;
-	/**
-	 * @var string|null
-	 */
+	/** @var string|null */
 	private $uid;
-	/**
-	 *@var IConfig
-	 */
+
+	/** @var IConfig */
 	private $config;
 
 	public function __construct(IUserManager $userManager,
-								TwoFactorManager $providerManager,
-								IAuthTokenProvider $tokenProvider,
 								ProviderLoader $providerLoader,
 								IUserSession $userSession,
-								ISession $session,
 								IConfig $config,
-								IInitialStateService $initialStateService,
 								?string $UserId) {
 		$this->userManager = $userManager;
-		$this->twoFactorManager = $providerManager;
-		$this->tokenProvider = $tokenProvider;
 		$this->providerLoader = $providerLoader;
 		$this->userSession = $userSession;
-		$this->session = $session;
-		$this->initialStateService = $initialStateService;
 		$this->uid = $UserId;
 		$this->config = $config;
 	}
 
-	/**
-	 * @return TemplateResponse returns the instance with all parameters set, ready to be rendered
-	 * @since 9.1
-	 */
-	public function getForm() {
+	public function getForm(): TemplateResponse {
 		$user = $this->userManager->get($this->uid);
 		$passwordChangeSupported = false;
 		if ($user !== null) {
 			$passwordChangeSupported = $user->canChangePassword();
 		}
 
-		$this->initialStateService->provideInitialState(
-			'settings',
-			'app_tokens',
-			$this->getAppTokens()
-		);
-
 		return new TemplateResponse('settings', 'settings/personal/security', [
 			'passwordChangeSupported' => $passwordChangeSupported,
 			'twoFactorProviderData' => $this->getTwoFactorProviderData(),
@@ -119,23 +87,11 @@ class Security implements ISettings {
 		]);
 	}
 
-	/**
-	 * @return string the section ID, e.g. 'sharing'
-	 * @since 9.1
-	 */
-	public function getSection() {
+	public function getSection(): string {
 		return 'security';
 	}
 
-	/**
-	 * @return int whether the form should be rather on the top or bottom of
-	 * the admin section. The forms are arranged in ascending order of the
-	 * priority values. It is required to return a value between 0 and 100.
-	 *
-	 * E.g.: 70
-	 * @since 9.1
-	 */
-	public function getPriority() {
+	public function getPriority(): int {
 		return 10;
 	}
 
@@ -157,32 +113,4 @@ class Security implements ISettings {
 			}))
 		];
 	}
-
-	private function getAppTokens(): array {
-		$tokens = $this->tokenProvider->getTokenByUser($this->uid);
-
-		try {
-			$sessionId = $this->session->getId();
-		} catch (SessionNotAvailableException $ex) {
-			return [];
-		}
-		try {
-			$sessionToken = $this->tokenProvider->getToken($sessionId);
-		} catch (InvalidTokenException $ex) {
-			return [];
-		}
-
-		return array_map(function (IToken $token) use ($sessionToken) {
-			$data = $token->jsonSerialize();
-			$data['canDelete'] = true;
-			$data['canRename'] = $token instanceof INamedToken;
-			if ($sessionToken->getId() === $token->getId()) {
-				$data['canDelete'] = false;
-				$data['canRename'] = false;
-				$data['current'] = true;
-			}
-			return $data;
-		}, $tokens);
-	}
-
 }

settings/Settings/Personal/Security/Authtokens.php

@@ -0,0 +1,107 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019, 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/>.
+ *
+ */
+
+namespace OC\Settings\Personal\Security;
+
+use function array_map;
+use OC\Authentication\Exceptions\InvalidTokenException;
+use OC\Authentication\Token\INamedToken;
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
+use OC\Authentication\Token\IToken;
+use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IInitialStateService;
+use OCP\ISession;
+use OCP\Session\Exceptions\SessionNotAvailableException;
+use OCP\Settings\ISettings;
+
+class Authtokens implements ISettings {
+
+	/** @var IAuthTokenProvider */
+	private $tokenProvider;
+
+	/** @var ISession */
+	private $session;
+
+	/** @var IInitialStateService */
+	private $initialStateService;
+
+	/** @var string|null */
+	private $uid;
+
+	public function __construct(IAuthTokenProvider $tokenProvider,
+								ISession $session,
+								IInitialStateService $initialStateService,
+								?string $UserId) {
+		$this->tokenProvider = $tokenProvider;
+		$this->session = $session;
+		$this->initialStateService = $initialStateService;
+		$this->uid = $UserId;
+	}
+
+	public function getForm(): TemplateResponse {
+		$this->initialStateService->provideInitialState(
+			'settings',
+			'app_tokens',
+			$this->getAppTokens()
+		);
+
+		return new TemplateResponse('settings', 'settings/personal/security/authtokens');
+	}
+
+	public function getSection(): string {
+		return 'security';
+	}
+
+	public function getPriority(): int {
+		return 100;
+	}
+
+	private function getAppTokens(): array {
+		$tokens = $this->tokenProvider->getTokenByUser($this->uid);
+
+		try {
+			$sessionId = $this->session->getId();
+		} catch (SessionNotAvailableException $ex) {
+			return [];
+		}
+		try {
+			$sessionToken = $this->tokenProvider->getToken($sessionId);
+		} catch (InvalidTokenException $ex) {
+			return [];
+		}
+
+		return array_map(function (IToken $token) use ($sessionToken) {
+			$data = $token->jsonSerialize();
+			$data['canDelete'] = true;
+			$data['canRename'] = $token instanceof INamedToken;
+			if ($sessionToken->getId() === $token->getId()) {
+				$data['canDelete'] = false;
+				$data['canRename'] = false;
+				$data['current'] = true;
+			}
+			return $data;
+		}, $tokens);
+	}
+
+}

settings/js/vue-settings-personal-security.js

@@ -904,5 +904,5 @@ function y(){throw new Error("Dynamic requires are not currently supported by ro
  * 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/>.
  */
-i.nc=btoa(OC.requestToken),r.default.use(a.a),r.default.use(s.a,{defaultHtml:!1}),r.default.prototype.t=t,new(r.default.extend(N))({propsData:{tokens:OCP.InitialState.loadState("settings","app_tokens")}}).$mount("#security")}]);
-//# sourceMappingURL=vue-settings-personal-security.js.map?v=bedaaf1d6e26dcd2fca0
+i.nc=btoa(OC.requestToken),r.default.use(a.a),r.default.use(s.a,{defaultHtml:!1}),r.default.prototype.t=t,new(r.default.extend(N))({propsData:{tokens:OCP.InitialState.loadState("settings","app_tokens")}}).$mount("#security-authtokens")}]);
+//# sourceMappingURL=vue-settings-personal-security.js.map?v=a2158eef5a0e4bafeee7

settings/src/main-personal-security.js

@@ -36,4 +36,4 @@ new View({
 	propsData: {
 		tokens: OCP.InitialState.loadState('settings', 'app_tokens'),
 	}
-}).$mount('#security');
+}).$mount('#security-authtokens');

settings/templates/settings/personal/security.php

@@ -104,4 +104,3 @@ if($_['passwordChangeSupported']) {
 	</ul>
 </div>
 
-<div id="security" class="section"></div>

settings/templates/settings/personal/security/authtokens.php

@@ -0,0 +1,31 @@
+<?php
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2019, 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/>.
+ *
+ */
+
+script('settings', [
+	'vue-settings-personal-security',
+]);
+
+?>
+
+<div id="security-authtokens" class="section"></div>

tests/Settings/Personal/Security/AuthtokensTest.php

@@ -0,0 +1,124 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * @copyright 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2019 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @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 Test\Settings\Personal\Security;
+
+use OC\Authentication\Token\DefaultToken;
+use OC\Authentication\Token\IProvider as IAuthTokenProvider;
+use OC\Settings\Personal\Security;
+use OC\Settings\Personal\Security\Authtokens;
+use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IInitialStateService;
+use OCP\ISession;
+use PHPUnit\Framework\MockObject\MockObject;
+use Test\TestCase;
+
+class AuthtokensTest extends TestCase {
+
+	/** @var IAuthTokenProvider|MockObject */
+	private $authTokenProvider;
+
+	/** @var ISession|MockObject */
+	private $session;
+
+	/** @var IInitialStateService|MockObject */
+	private $initialStateService;
+
+	/** @var string */
+	private $uid;
+
+	/** @var Security\Authtokens */
+	private $section;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
+		$this->session = $this->createMock(ISession::class);
+		$this->initialStateService = $this->createMock(IInitialStateService::class);
+		$this->uid = 'test123';
+
+		$this->section = new Authtokens(
+			$this->authTokenProvider,
+			$this->session,
+			$this->initialStateService,
+			$this->uid
+		);
+	}
+
+	public function testGetForm() {
+		$token1 = new DefaultToken();
+		$token1->setId(100);
+		$token2 = new DefaultToken();
+		$token2->setId(200);
+		$tokens = [
+			$token1,
+			$token2,
+		];
+		$sessionToken = new DefaultToken();
+		$sessionToken->setId(100);
+
+		$this->authTokenProvider->expects($this->once())
+			->method('getTokenByUser')
+			->with($this->uid)
+			->willReturn($tokens);
+		$this->session->expects($this->once())
+			->method('getId')
+			->willReturn('session123');
+		$this->authTokenProvider->expects($this->once())
+			->method('getToken')
+			->with('session123')
+			->willReturn($sessionToken);
+		$this->initialStateService->expects($this->once())
+			->method('provideInitialState')
+			->with('settings', 'app_tokens', [
+				[
+					'id' => 100,
+					'name' => null,
+					'lastActivity' => 0,
+					'type' => 0,
+					'canDelete' => false,
+					'current' => true,
+					'scope' => ['filesystem' => true],
+					'canRename' => false,
+				],
+				[
+					'id' => 200,
+					'name' => null,
+					'lastActivity' => 0,
+					'type' => 0,
+					'canDelete' => true,
+					'scope' => ['filesystem' => true],
+					'canRename' => true,
+				],
+			]);
+
+		$form = $this->section->getForm();
+
+		$expected = new TemplateResponse('settings', 'settings/personal/security/authtokens');
+		$this->assertEquals($expected, $form);
+	}
+
+}

tests/lib/Settings/ManagerTest.php

@@ -244,15 +244,24 @@ class ManagerTest extends TestCase {
 		$section->expects($this->once())
 			->method('getPriority')
 			->willReturn(16);
-		$this->container->expects($this->once())
+		$section2 = $this->createMock(Security\Authtokens::class);
+		$section2->expects($this->once())
+			->method('getPriority')
+			->willReturn(100);
+		$this->container->expects($this->at(0))
 			->method('query')
 			->with(Security::class)
 			->willReturn($section);
+		$this->container->expects($this->at(1))
+			->method('query')
+			->with(Security\Authtokens::class)
+			->willReturn($section2);
 
 		$settings = $this->manager->getPersonalSettings('security');
 
 		$this->assertEquals([
-			16 => [$section]
+			16 => [$section],
+			100 => [$section2],
 		], $settings);
 	}
 

tests/lib/Settings/Personal/SecurityTest.php

@@ -1,5 +1,4 @@
 <?php
-
 declare(strict_types=1);
 
 /**
@@ -25,15 +24,10 @@ declare(strict_types=1);
 
 namespace Test\Settings\Personal;
 
-use OC\Authentication\Token\DefaultToken;
-use OC\Authentication\Token\IProvider as IAuthTokenProvider;
-use OC\Authentication\TwoFactorAuth\Manager as TwoFactorManager;
 use OC\Authentication\TwoFactorAuth\ProviderLoader;
 use OC\Settings\Personal\Security;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
-use OCP\IInitialStateService;
-use OCP\ISession;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\IUserSession;
@@ -45,27 +39,15 @@ class SecurityTest extends TestCase {
 	/** @var IUserManager|MockObject */
 	private $userManager;
 
-	/** @var TwoFactorManager|MockObject */
-	private $twoFactorManager;
-
-	/** @var IAuthTokenProvider|MockObject */
-	private $authTokenProvider;
-
 	/** @var ProviderLoader|MockObject */
 	private $providerLoader;
 
 	/** @var IUserSession|MockObject */
 	private $userSession;
 
-	/** @var ISession|MockObject */
-	private $session;
-
 	/** @var IConfig|MockObject */
 	private $config;
 
-	/** @var IInitialStateService|MockObject */
-	private $initialStateService;
-
 	/** @var string */
 	private $uid;
 
@@ -76,39 +58,21 @@ class SecurityTest extends TestCase {
 		parent::setUp();
 
 		$this->userManager = $this->createMock(IUserManager::class);
-		$this->twoFactorManager = $this->createMock(TwoFactorManager::class);
-		$this->authTokenProvider = $this->createMock(IAuthTokenProvider::class);
 		$this->providerLoader = $this->createMock(ProviderLoader::class);
 		$this->userSession = $this->createMock(IUserSession::class);
 		$this->config = $this->createMock(IConfig::class);
-		$this->session = $this->createMock(ISession::class);
-		$this->initialStateService = $this->createMock(IInitialStateService::class);
 		$this->uid = 'test123';
 
 		$this->section = new Security(
 			$this->userManager,
-			$this->twoFactorManager,
-			$this->authTokenProvider,
 			$this->providerLoader,
 			$this->userSession,
-			$this->session,
 			$this->config,
-			$this->initialStateService,
 			$this->uid
 		);
 	}
 
 	public function testGetForm() {
-		$token1 = new DefaultToken();
-		$token1->setId(100);
-		$token2 = new DefaultToken();
-		$token2->setId(200);
-		$tokens = [
-			$token1,
-			$token2,
-		];
-		$sessionToken = new DefaultToken();
-		$sessionToken->setId(100);
 		$user = $this->createMock(IUser::class);
 		$this->userManager->expects($this->once())
 			->method('get')
@@ -117,40 +81,6 @@ class SecurityTest extends TestCase {
 		$user->expects($this->once())
 			->method('canChangePassword')
 			->willReturn(true);
-		$this->authTokenProvider->expects($this->once())
-			->method('getTokenByUser')
-			->with($this->uid)
-			->willReturn($tokens);
-		$this->session->expects($this->once())
-			->method('getId')
-			->willReturn('session123');
-		$this->authTokenProvider->expects($this->once())
-			->method('getToken')
-			->with('session123')
-			->willReturn($sessionToken);
-		$this->initialStateService->expects($this->once())
-			->method('provideInitialState')
-			->with('settings', 'app_tokens', [
-				[
-					'id' => 100,
-					'name' => null,
-					'lastActivity' => 0,
-					'type' => 0,
-					'canDelete' => false,
-					'current' => true,
-					'scope' => ['filesystem' => true],
-					'canRename' => false,
-				],
-				[
-					'id' => 200,
-					'name' => null,
-					'lastActivity' => 0,
-					'type' => 0,
-					'canDelete' => true,
-					'scope' => ['filesystem' => true],
-					'canRename' => true,
-				],
-			]);
 		$this->userSession->expects($this->once())
 			->method('getUser')
 			->willReturn($user);