Merge pull request #32503 from nextcloud/improve-two-factor

Improve two factor admin settings

4 files changed, 43 insertions, 36 deletions

diff --git a/apps/settings/lib/Settings/Admin/Security.php b/apps/settings/lib/Settings/Admin/Security.php
index 2580c0a3d00..f84ef03b61b 100644
--- a/apps/settings/lib/Settings/Admin/Security.php
+++ b/apps/settings/lib/Settings/Admin/Security.php
@@ -31,30 +31,26 @@ use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\Encryption\IManager;
 use OCP\IUserManager;
+use OCP\IURLGenerator;
 use OCP\Settings\ISettings;
 
 class Security implements ISettings {
-
-	/** @var IManager */
-	private $manager;
-
-	/** @var IUserManager */
-	private $userManager;
-
-	/** @var MandatoryTwoFactor */
-	private $mandatoryTwoFactor;
-
-	/** @var IInitialState */
-	private $initialState;
+	private IManager $manager;
+	private IUserManager $userManager;
+	private MandatoryTwoFactor $mandatoryTwoFactor;
+	private IInitialState $initialState;
+	private IURLGenerator $urlGenerator;
 
 	public function __construct(IManager $manager,
 								IUserManager $userManager,
 								MandatoryTwoFactor $mandatoryTwoFactor,
-								IInitialState $initialState) {
+								IInitialState $initialState,
+								IURLGenerator $urlGenerator) {
 		$this->manager = $manager;
 		$this->userManager = $userManager;
 		$this->mandatoryTwoFactor = $mandatoryTwoFactor;
 		$this->initialState = $initialState;
+		$this->urlGenerator = $urlGenerator;
 	}
 
 	/**
@@ -77,6 +73,11 @@ class Security implements ISettings {
 			$this->mandatoryTwoFactor->getState()
 		);
 
+		$this->initialState->provideInitialState(
+			'two-factor-admin-doc',
+			$this->urlGenerator->linkToDocs('admin-2fa')
+		);
+
 		$parameters = [
 			// Encryption API
 			'encryptionEnabled' => $this->manager->isEnabled(),
diff --git a/apps/settings/src/components/AdminTwoFactor.vue b/apps/settings/src/components/AdminTwoFactor.vue
index bfec05e331b..435348f30ab 100644
--- a/apps/settings/src/components/AdminTwoFactor.vue
+++ b/apps/settings/src/components/AdminTwoFactor.vue
@@ -1,23 +1,21 @@
 <template>
-	<div>
-		<p class="settings-hint">
-			{{ t('settings', 'Two-factor authentication can be enforced for all users and specific groups. If they do not have a two-factor provider configured, they will be unable to log into the system.') }}
-		</p>
+	<SettingsSection :title="t('settings', 'Two-Factor Authentication')"
+		:description="t('settings', 'Two-factor authentication can be enforced for all users and specific groups. If they do not have a two-factor provider configured, they will be unable to log into the system.')"
+		:doc-url="twoFactorAdminDoc">
 		<p v-if="loading">
 			<span class="icon-loading-small two-factor-loading" />
 			<span>{{ t('settings', 'Enforce two-factor authentication') }}</span>
 		</p>
-		<p v-else>
-			<input id="two-factor-enforced"
-				v-model="enforced"
-				type="checkbox"
-				class="checkbox">
-			<label for="two-factor-enforced">{{ t('settings', 'Enforce two-factor authentication') }}</label>
-		</p>
+		<CheckboxRadioSwitch v-else
+			id="two-factor-enforced"
+			:checked.sync="enforced"
+			type="switch">
+			{{ t('settings', 'Enforce two-factor authentication') }}
+		</CheckboxRadioSwitch>
 		<template v-if="enforced">
 			<h3>{{ t('settings', 'Limit to groups') }}</h3>
 			{{ t('settings', 'Enforcement of two-factor authentication can be set for certain groups only.') }}
-			<p>
+			<p class="top-margin">
 				{{ t('settings', 'Two-factor authentication is enforced for all members of the following groups.') }}
 			</p>
 			<p>
@@ -32,7 +30,7 @@
 					:close-on-select="false"
 					@search-change="searchGroup" />
 			</p>
-			<p>
+			<p class="top-margin">
 				{{ t('settings', 'Two-factor authentication is not enforced for members of the following groups.') }}
 			</p>
 			<p>
@@ -47,14 +45,14 @@
 					:close-on-select="false"
 					@search-change="searchGroup" />
 			</p>
-			<p>
+			<p class="top-margin">
 				<em>
 					<!-- this text is also found in the documentation. update it there as well if it ever changes -->
 					{{ t('settings', 'When groups are selected/excluded, they use the following logic to determine if a user has 2FA enforced: If no groups are selected, 2FA is enabled for everyone except members of the excluded groups. If groups are selected, 2FA is enabled for all members of these. If a user is both in a selected and excluded group, the selected takes precedence and 2FA is enforced.') }}
 				</em>
 			</p>
 		</template>
-		<p>
+		<p class="top-margin">
 			<Button v-if="dirty"
 				type="primary"
 				:disabled="loading"
@@ -62,13 +60,16 @@
 				{{ t('settings', 'Save changes') }}
 			</Button>
 		</p>
-	</div>
+	</SettingsSection>
 </template>
 
 <script>
 import axios from '@nextcloud/axios'
 import Multiselect from '@nextcloud/vue/dist/Components/Multiselect'
 import Button from '@nextcloud/vue/dist/Components/Button'
+import CheckboxRadioSwitch from '@nextcloud/vue/dist/Components/CheckboxRadioSwitch'
+import SettingsSection from '@nextcloud/vue/dist/Components/SettingsSection'
+import { loadState } from '@nextcloud/initial-state'
 
 import _ from 'lodash'
 import { generateUrl, generateOcsUrl } from '@nextcloud/router'
@@ -78,6 +79,8 @@ export default {
 	components: {
 		Multiselect,
 		Button,
+		CheckboxRadioSwitch,
+		SettingsSection,
 	},
 	data() {
 		return {
@@ -85,6 +88,7 @@ export default {
 			dirty: false,
 			groups: [],
 			loadingGroups: false,
+			twoFactorAdminDoc: loadState('settings', 'two-factor-admin-doc'),
 		}
 	},
 	computed: {
@@ -159,11 +163,15 @@ export default {
 }
 </script>
 
-<style>
+<style scoped>
 	.two-factor-loading {
 		display: inline-block;
 		vertical-align: sub;
 		margin-left: -2px;
 		margin-right: 1px;
 	}
+
+	.top-margin {
+		margin-top: 0.5rem;
+	}
 </style>
diff --git a/apps/settings/templates/settings/admin/security.php b/apps/settings/templates/settings/admin/security.php
index f0689b948af..e285e393e20 100644
--- a/apps/settings/templates/settings/admin/security.php
+++ b/apps/settings/templates/settings/admin/security.php
@@ -28,11 +28,7 @@ script('settings', 'vue-settings-admin-security');
 
 ?>
 
-<div id="two-factor-auth" class="section">
-	<h2><?php p($l->t('Two-Factor Authentication'));?></h2>
-	<a target="_blank" rel="noreferrer" class="icon-info" title="<?php p($l->t('Open documentation'));?>" href="<?php p(link_to_docs('admin-2fa')); ?>"></a>
-	<div id="two-factor-auth-settings"></div>
-</div>
+<div id="two-factor-auth-settings"></div>
 
 <div class="section" id='encryptionAPI'>
 	<h2><?php p($l->t('Server-side encryption')); ?></h2>
diff --git a/apps/settings/tests/Settings/Admin/SecurityTest.php b/apps/settings/tests/Settings/Admin/SecurityTest.php
index da4ce62d9cd..8bb330469fb 100644
--- a/apps/settings/tests/Settings/Admin/SecurityTest.php
+++ b/apps/settings/tests/Settings/Admin/SecurityTest.php
@@ -33,6 +33,7 @@ use OCA\Settings\Settings\Admin\Security;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Services\IInitialState;
 use OCP\IUserManager;
+use OCP\IURLGenerator;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
@@ -59,7 +60,8 @@ class SecurityTest extends TestCase {
 			$this->manager,
 			$this->userManager,
 			$this->mandatoryTwoFactor,
-			$this->initialState
+			$this->initialState,
+			$this->createMock(IURLGenerator::class)
 		);
 	}