Merge pull request #11765 from nextcloud/feature/mandatory-2fa-for-groups

Mandatory 2FA for groups

5 files changed, 277 insertions, 56 deletions

diff --git a/tests/Core/Command/TwoFactorAuth/EnforceTest.php b/tests/Core/Command/TwoFactorAuth/EnforceTest.php
index 8d896f2f45c..2d9b77e18c2 100644
--- a/tests/Core/Command/TwoFactorAuth/EnforceTest.php
+++ b/tests/Core/Command/TwoFactorAuth/EnforceTest.php
@@ -26,6 +26,7 @@ declare(strict_types=1);
 
 namespace Tests\Core\Command\TwoFactorAuth;
 
+use OC\Authentication\TwoFactorAuth\EnforcementState;
 use OC\Authentication\TwoFactorAuth\MandatoryTwoFactor;
 use OC\Core\Command\TwoFactorAuth\Enforce;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -51,11 +52,11 @@ class EnforceTest extends TestCase {
 
 	public function testEnforce() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('setEnforced')
-			->with(true);
+			->method('setState')
+			->with($this->equalTo(new EnforcementState(true)));
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
-			->willReturn(true);
+			->method('getState')
+			->willReturn(new EnforcementState(true));
 
 		$rc = $this->command->execute([
 			'--on' => true,
@@ -66,13 +67,49 @@ class EnforceTest extends TestCase {
 		$this->assertContains("Two-factor authentication is enforced for all users", $display);
 	}
 
+	public function testEnforceForOneGroup() {
+		$this->mandatoryTwoFactor->expects($this->once())
+			->method('setState')
+			->with($this->equalTo(new EnforcementState(true, ['twofactorers'])));
+		$this->mandatoryTwoFactor->expects($this->once())
+			->method('getState')
+			->willReturn(new EnforcementState(true, ['twofactorers']));
+
+		$rc = $this->command->execute([
+			'--on' => true,
+			'--group' => ['twofactorers'],
+		]);
+
+		$this->assertEquals(0, $rc);
+		$display = $this->command->getDisplay();
+		$this->assertContains("Two-factor authentication is enforced for members of the group(s) twofactorers", $display);
+	}
+
+	public function testEnforceForAllExceptOneGroup() {
+		$this->mandatoryTwoFactor->expects($this->once())
+			->method('setState')
+			->with($this->equalTo(new EnforcementState(true, [], ['yoloers'])));
+		$this->mandatoryTwoFactor->expects($this->once())
+			->method('getState')
+			->willReturn(new EnforcementState(true, [], ['yoloers']));
+
+		$rc = $this->command->execute([
+			'--on' => true,
+			'--exclude' => ['yoloers'],
+		]);
+
+		$this->assertEquals(0, $rc);
+		$display = $this->command->getDisplay();
+		$this->assertContains("Two-factor authentication is enforced for all users, except members of yoloers", $display);
+	}
+
 	public function testDisableEnforced() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('setEnforced')
-			->with(false);
+			->method('setState')
+			->with(new EnforcementState(false));
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
-			->willReturn(false);
+			->method('getState')
+			->willReturn(new EnforcementState(false));
 
 		$rc = $this->command->execute([
 			'--off' => true,
@@ -85,8 +122,8 @@ class EnforceTest extends TestCase {
 
 	public function testCurrentStateEnabled() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
-			->willReturn(true);
+			->method('getState')
+			->willReturn(new EnforcementState(true));
 
 		$rc = $this->command->execute([]);
 
@@ -97,8 +134,8 @@ class EnforceTest extends TestCase {
 
 	public function testCurrentStateDisabled() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
-			->willReturn(false);
+			->method('getState')
+			->willReturn(new EnforcementState(false));
 
 		$rc = $this->command->execute([]);
 
diff --git a/tests/Settings/Controller/TwoFactorSettingsControllerTest.php b/tests/Settings/Controller/TwoFactorSettingsControllerTest.php
index 353fc759425..6872d4e2152 100644
--- a/tests/Settings/Controller/TwoFactorSettingsControllerTest.php
+++ b/tests/Settings/Controller/TwoFactorSettingsControllerTest.php
@@ -22,6 +22,7 @@
 
 namespace Tests\Settings\Controller;
 
+use OC\Authentication\TwoFactorAuth\EnforcementState;
 use OC\Authentication\TwoFactorAuth\MandatoryTwoFactor;
 use OC\Settings\Controller\TwoFactorSettingsController;
 use OCP\AppFramework\Http\JSONResponse;
@@ -54,12 +55,11 @@ class TwoFactorSettingsControllerTest extends TestCase {
 	}
 
 	public function testIndex() {
+		$state = new EnforcementState(true);
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
-			->willReturn(true);
-		$expected = new JSONResponse([
-			'enabled' => true,
-		]);
+			->method('getState')
+			->willReturn($state);
+		$expected = new JSONResponse($state);
 
 		$resp = $this->controller->index();
 
@@ -67,12 +67,14 @@ class TwoFactorSettingsControllerTest extends TestCase {
 	}
 
 	public function testUpdate() {
+		$state = new EnforcementState(true);
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('setEnforced')
-			->with(true);
-		$expected = new JSONResponse([
-			'enabled' => true,
-		]);
+			->method('setState')
+			->with($this->equalTo(new EnforcementState(true)));
+		$this->mandatoryTwoFactor->expects($this->once())
+			->method('getState')
+			->willReturn($state);
+		$expected = new JSONResponse($state);
 
 		$resp = $this->controller->update(true);
 
diff --git a/tests/lib/Authentication/TwoFactorAuth/EnforcementStateTest.php b/tests/lib/Authentication/TwoFactorAuth/EnforcementStateTest.php
new file mode 100644
index 00000000000..0508c84787c
--- /dev/null
+++ b/tests/lib/Authentication/TwoFactorAuth/EnforcementStateTest.php
@@ -0,0 +1,67 @@
+<?php
+/**
+ * @copyright 2018 Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @author 2018 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/>.
+ */
+
+/**
+ * Created by PhpStorm.
+ * User: christoph
+ * Date: 11.10.18
+ * Time: 13:01
+ */
+
+namespace Tests\Authentication\TwoFactorAuth;
+
+use OC\Authentication\TwoFactorAuth\EnforcementState;
+use Test\TestCase;
+
+class EnforcementStateTest extends TestCase {
+
+	public function testIsEnforced() {
+		$state = new EnforcementState(true);
+
+		$this->assertTrue($state->isEnforced());
+	}
+
+	public function testGetEnforcedGroups() {
+		$state = new EnforcementState(true, ['twofactorers']);
+
+		$this->assertEquals(['twofactorers'], $state->getEnforcedGroups());
+	}
+
+	public function testGetExcludedGroups() {
+		$state = new EnforcementState(true, [], ['yoloers']);
+
+		$this->assertEquals(['yoloers'], $state->getExcludedGroups());
+	}
+
+	public function testJsonSerialize() {
+		$state = new EnforcementState(true, ['twofactorers'], ['yoloers']);
+		$expected = [
+			'enforced' => true,
+			'enforcedGroups' => ['twofactorers'],
+			'excludedGroups' => ['yoloers'],
+		];
+
+		$json = $state->jsonSerialize();
+
+		$this->assertEquals($expected, $json);
+	}
+}
diff --git a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
index acc0f0d3e92..0f09691bc1c 100644
--- a/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
@@ -37,58 +37,59 @@ use OCP\IConfig;
 use OCP\ILogger;
 use OCP\ISession;
 use OCP\IUser;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\EventDispatcher\EventDispatcherInterface;
 use Test\TestCase;
 
 class ManagerTest extends TestCase {
 
-	/** @var IUser|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IUser|MockObject */
 	private $user;
 
-	/** @var ProviderLoader|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ProviderLoader|MockObject */
 	private $providerLoader;
 
-	/** @var IRegistry|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IRegistry|MockObject */
 	private $providerRegistry;
 
-	/** @var MandatoryTwoFactor|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var MandatoryTwoFactor|MockObject */
 	private $mandatoryTwoFactor;
 
-	/** @var ISession|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ISession|MockObject */
 	private $session;
 
 	/** @var Manager */
 	private $manager;
 
-	/** @var IConfig|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IConfig|MockObject */
 	private $config;
 
-	/** @var IManager|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IManager|MockObject */
 	private $activityManager;
 
-	/** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ILogger|MockObject */
 	private $logger;
 
-	/** @var IProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IProvider|MockObject */
 	private $fakeProvider;
 
-	/** @var IProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var IProvider|MockObject */
 	private $backupProvider;
 
-	/** @var TokenProvider|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var TokenProvider|MockObject */
 	private $tokenProvider;
 
-	/** @var ITimeFactory|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var ITimeFactory|MockObject */
 	private $timeFactory;
 
-	/** @var EventDispatcherInterface|\PHPUnit_Framework_MockObject_MockObject */
+	/** @var EventDispatcherInterface|MockObject */
 	private $eventDispatcher;
 
 	protected function setUp() {
 		parent::setUp();
 
 		$this->user = $this->createMock(IUser::class);
-		$this->providerLoader = $this->createMock(\OC\Authentication\TwoFactorAuth\ProviderLoader::class);
+		$this->providerLoader = $this->createMock(ProviderLoader::class);
 		$this->providerRegistry = $this->createMock(IRegistry::class);
 		$this->mandatoryTwoFactor = $this->createMock(MandatoryTwoFactor::class);
 		$this->session = $this->createMock(ISession::class);
@@ -150,7 +151,8 @@ class ManagerTest extends TestCase {
 
 	public function testIsTwoFactorAuthenticatedEnforced() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
+			->method('isEnforcedFor')
+			->with($this->user)
 			->willReturn(true);
 
 		$enabled = $this->manager->isTwoFactorAuthenticated($this->user);
@@ -160,7 +162,8 @@ class ManagerTest extends TestCase {
 
 	public function testIsTwoFactorAuthenticatedNoProviders() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
+			->method('isEnforcedFor')
+			->with($this->user)
 			->willReturn(false);
 		$this->providerRegistry->expects($this->once())
 			->method('getProviderStates')
@@ -174,7 +177,8 @@ class ManagerTest extends TestCase {
 
 	public function testIsTwoFactorAuthenticatedOnlyBackupCodes() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
+			->method('isEnforcedFor')
+			->with($this->user)
 			->willReturn(false);
 		$this->providerRegistry->expects($this->once())
 			->method('getProviderStates')
@@ -196,7 +200,8 @@ class ManagerTest extends TestCase {
 
 	public function testIsTwoFactorAuthenticatedFailingProviders() {
 		$this->mandatoryTwoFactor->expects($this->once())
-			->method('isEnforced')
+			->method('isEnforcedFor')
+			->with($this->user)
 			->willReturn(false);
 		$this->providerRegistry->expects($this->once())
 			->method('getProviderStates')
diff --git a/tests/lib/Authentication/TwoFactorAuth/MandatoryTwoFactorTest.php b/tests/lib/Authentication/TwoFactorAuth/MandatoryTwoFactorTest.php
index 1cacbd5f787..61ffb404dd9 100644
--- a/tests/lib/Authentication/TwoFactorAuth/MandatoryTwoFactorTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/MandatoryTwoFactorTest.php
@@ -26,8 +26,11 @@ declare(strict_types=1);
 
 namespace Tests\Authentication\TwoFactorAuth;
 
+use OC\Authentication\TwoFactorAuth\EnforcementState;
 use OC\Authentication\TwoFactorAuth\MandatoryTwoFactor;
 use OCP\IConfig;
+use OCP\IGroupManager;
+use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
@@ -36,6 +39,9 @@ class MandatoryTwoFactorTest extends TestCase {
 	/** @var IConfig|MockObject */
 	private $config;
 
+	/** @var IGroupManager|MockObject */
+	private $groupManager;
+
 	/** @var MandatoryTwoFactor */
 	private $mandatoryTwoFactor;
 
@@ -43,46 +49,150 @@ class MandatoryTwoFactorTest extends TestCase {
 		parent::setUp();
 
 		$this->config = $this->createMock(IConfig::class);
+		$this->groupManager = $this->createMock(IGroupManager::class);
 
-		$this->mandatoryTwoFactor = new MandatoryTwoFactor($this->config);
+		$this->mandatoryTwoFactor = new MandatoryTwoFactor($this->config, $this->groupManager);
 	}
 
 	public function testIsNotEnforced() {
-		$this->config->expects($this->once())
+		$this->config
 			->method('getSystemValue')
-			->with('twofactor_enforced', 'false')
-			->willReturn('false');
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'false'],
+				['twofactor_enforced_groups', [], []],
+				['twofactor_enforced_excluded_groups', [], []],
+			]);
 
-		$isEnforced = $this->mandatoryTwoFactor->isEnforced();
+		$state = $this->mandatoryTwoFactor->getState();
 
-		$this->assertFalse($isEnforced);
+		$this->assertFalse($state->isEnforced());
 	}
 
 	public function testIsEnforced() {
-		$this->config->expects($this->once())
+		$this->config
+			->method('getSystemValue')
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'true'],
+				['twofactor_enforced_groups', [], []],
+				['twofactor_enforced_excluded_groups', [], []],
+			]);
+
+		$state = $this->mandatoryTwoFactor->getState();
+
+		$this->assertTrue($state->isEnforced());
+	}
+
+	public function testIsNotEnforcedForAnybody() {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user123');
+		$this->config
 			->method('getSystemValue')
-			->with('twofactor_enforced', 'false')
-			->willReturn('true');
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'false'],
+				['twofactor_enforced_groups', [], []],
+				['twofactor_enforced_excluded_groups', [], []],
+			]);
 
-		$isEnforced = $this->mandatoryTwoFactor->isEnforced();
+		$isEnforced = $this->mandatoryTwoFactor->isEnforcedFor($user);
+
+		$this->assertFalse($isEnforced);
+	}
+
+	public function testIsEnforcedForAGroupMember() {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user123');
+		$this->config
+			->method('getSystemValue')
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'true'],
+				['twofactor_enforced_groups', [], ['twofactorers']],
+				['twofactor_enforced_excluded_groups', [], []],
+			]);
+		$this->groupManager->method('isInGroup')
+			->willReturnCallback(function($user, $group) {
+				return $user === 'user123' && $group ==='twofactorers';
+			});
+
+		$isEnforced = $this->mandatoryTwoFactor->isEnforcedFor($user);
 
 		$this->assertTrue($isEnforced);
 	}
 
+	public function testIsEnforcedForOtherGroups() {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user123');
+		$this->config
+			->method('getSystemValue')
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'true'],
+				['twofactor_enforced_groups', [], ['twofactorers']],
+				['twofactor_enforced_excluded_groups', [], []],
+			]);
+		$this->groupManager->method('isInGroup')
+			->willReturn(false);
+
+		$isEnforced = $this->mandatoryTwoFactor->isEnforcedFor($user);
+
+		$this->assertFalse($isEnforced);
+	}
+
+	public function testIsEnforcedButMemberOfExcludedGroup() {
+		$user = $this->createMock(IUser::class);
+		$user->method('getUID')->willReturn('user123');
+		$this->config
+			->method('getSystemValue')
+			->willReturnMap([
+				['twofactor_enforced', 'false', 'true'],
+				['twofactor_enforced_groups', [], []],
+				['twofactor_enforced_excluded_groups', [], ['yoloers']],
+			]);
+		$this->groupManager->method('isInGroup')
+			->willReturnCallback(function($user, $group) {
+				return $user === 'user123' && $group ==='yoloers';
+			});
+
+		$isEnforced = $this->mandatoryTwoFactor->isEnforcedFor($user);
+
+		$this->assertFalse($isEnforced);
+	}
+
 	public function testSetEnforced() {
-		$this->config->expects($this->once())
+		$this->config
+			->expects($this->exactly(3))
+			->method('setSystemValue')
+			->willReturnMap([
+				['twofactor_enforced', 'true'],
+				['twofactor_enforced_groups', []],
+				['twofactor_enforced_excluded_groups', []],
+			]);
+
+		$this->mandatoryTwoFactor->setState(new EnforcementState(true));
+	}
+
+	public function testSetEnforcedForGroups() {
+		$this->config
+			->expects($this->exactly(3))
 			->method('setSystemValue')
-			->with('twofactor_enforced', 'true');
+			->willReturnMap([
+				['twofactor_enforced', 'true'],
+				['twofactor_enforced_groups', ['twofactorers']],
+				['twofactor_enforced_excluded_groups', ['yoloers']],
+			]);
 
-		$this->mandatoryTwoFactor->setEnforced(true);
+		$this->mandatoryTwoFactor->setState(new EnforcementState(true, ['twofactorers'], ['yoloers']));
 	}
 
 	public function testSetNotEnforced() {
-		$this->config->expects($this->once())
+		$this->config
+			->expects($this->exactly(3))
 			->method('setSystemValue')
-			->with('twofactor_enforced', 'false');
+			->willReturnMap([
+				['twofactor_enforced', 'false'],
+				['twofactor_enforced_groups', []],
+				['twofactor_enforced_excluded_groups', []],
+			]);
 
-		$this->mandatoryTwoFactor->setEnforced(false);
+		$this->mandatoryTwoFactor->setState(new EnforcementState(false));
 	}
 
 }