Add config option to turn on Collabora feature lock for read only users

Signed-off-by: Julius Härtl <jus@bitgrid.net>
2022-10-14 09:30:05 +02:00 · 2022-10-14 09:30:05 +02:00 · be5ef8f35d
--- a/lib/AppConfig.php
+++ b/lib/AppConfig.php
@ -22,6 +22,8 @@ class AppConfig {

 	public const SYSTEM_GS_TRUSTED_HOSTS = 'gs.trustedHosts';

+	public const READ_ONLY_FEATURE_LOCK = 'read_only_feature_lock';
+
 	private $defaults = [
 		'wopi_url' => '',
 		'timeout' => 15,
@ -134,4 +136,30 @@ class AppConfig {
 	public function getCollaboraUrlInternal(): string {
 		return $this->config->getAppValue(Application::APPNAME, self::WOPI_URL, '');
 	}
+
+	public function getUseGroups(): ?array {
+		$groups = $this->config->getAppValue(Application::APPNAME, 'use_groups', '');
+		if ($groups === '') {
+			return null;
+		}
+
+		return $this->splitGroups($groups);
+	}
+
+	public function getEditGroups(): ?array {
+		$groups = $this->config->getAppValue(Application::APPNAME, 'edit_groups', '');
+		if ($groups === '') {
+			return null;
+		}
+
+		return $this->splitGroups($groups);
+	}
+
+	public function isReadOnlyFeatureLocked(): bool {
+		return $this->config->getAppValue(Application::APPNAME, self::READ_ONLY_FEATURE_LOCK, 'no') === 'yes';
+	}
+
+	private function splitGroups(string $groupString): array {
+		return explode('|', $groupString);
+	}
 }
--- a/lib/Controller/WopiController.php
+++ b/lib/Controller/WopiController.php
@ -30,6 +30,7 @@ use OCA\Richdocuments\Events\DocumentOpenedEvent;
 use OCA\Richdocuments\Exceptions\ExpiredTokenException;
 use OCA\Richdocuments\Exceptions\UnknownTokenException;
 use OCA\Richdocuments\Helper;
+use OCA\Richdocuments\PermissionManager;
 use OCA\Richdocuments\Service\FederationService;
 use OCA\Richdocuments\Service\UserScopeService;
 use OCA\Richdocuments\TemplateManager;
@ -80,6 +81,8 @@ class WopiController extends Controller {
 	private $appConfig;
 	/** @var TokenManager */
 	private $tokenManager;
+	/** @var PermissionManager */
+	private $permissionManager;
 	/** @var IUserManager */
 	private $userManager;
 	/** @var WopiMapper */
@ -114,6 +117,7 @@ class WopiController extends Controller {
 		IConfig $config,
 		AppConfig $appConfig,
 		TokenManager $tokenManager,
+		PermissionManager $permissionManager,
 		IUserManager $userManager,
 		WopiMapper $wopiMapper,
 		ILogger $logger,
@ -132,6 +136,7 @@ class WopiController extends Controller {
 		$this->config = $config;
 		$this->appConfig = $appConfig;
 		$this->tokenManager = $tokenManager;
+		$this->permissionManager = $permissionManager;
 		$this->userManager = $userManager;
 		$this->wopiMapper = $wopiMapper;
 		$this->logger = $logger;
@ -215,6 +220,7 @@ class WopiController extends Controller {
 			'HidePrintOption' => $wopi->getHideDownload(),
 			'DownloadAsPostMessage' => $wopi->getDirect(),
 			'SupportsLocks' => $this->lockManager->isLockProviderAvailable(),
+			'IsUserLocked' => $this->permissionManager->userIsFeatureLocked($wopi->getEditorUid()),
 		];

 		if ($wopi->hasTemplateId()) {
--- a/lib/PermissionManager.php
+++ b/lib/PermissionManager.php
@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
 * @copyright Copyright (c) 2017 Lukas Reschke <lukas@statuscode.ch>
 *
@ -21,38 +23,29 @@

 namespace OCA\Richdocuments;

-use OCA\Richdocuments\AppInfo\Application;
-use OCP\IConfig;
 use OCP\IGroupManager;
+use OCP\IUserManager;
 use OCP\IUserSession;

 class PermissionManager {
-	/** @var IConfig */
-	private $config;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IUserSession */
-	private $userSession;
+	private AppConfig $config;
+	private IGroupManager $groupManager;
+	private IUserManager $userManager;
+	private IUserSession $userSession;

 	public function __construct(
-		IConfig $config,
+		AppConfig $config,
 		IGroupManager $groupManager,
+		IUserManager $userManager,
 		IUserSession $userSession
 	) {
 		$this->config = $config;
 		$this->groupManager = $groupManager;
+		$this->userManager = $userManager;
 		$this->userSession = $userSession;
 	}

-	/**
-	 * @param string $groupString
-	 * @return array
-	 */
-	private function splitGroups($groupString) {
-		return explode('|', $groupString);
-	}
-
-	public function isEnabledForUser(string $userId = null) {
+	private function userMatchesGroupList(?string $userId = null, ?array $groupList = []): bool {
 		if ($userId === null) {
 			$user = $this->userSession->getUser();
 			$userId = $user ? $user->getUID() : null;
@ -62,22 +55,46 @@ class PermissionManager {
 			return true;
 		}

-		$enabledForGroups = $this->config->getAppValue(Application::APPNAME, 'use_groups', '');
-		if ($enabledForGroups === '') {
+		if ($groupList === null || $groupList === []) {
 			return true;
 		}

-		$groups = $this->splitGroups($enabledForGroups);
-		foreach ($groups as $group) {
-			if ($this->groupManager->isInGroup($userId, $group)) {
-				return true;
-			}
-		}
-
 		if ($this->groupManager->isAdmin($userId)) {
 			return true;
 		}

+		$userGroups = $this->groupManager->getUserGroupIds($this->userManager->get($userId));
+
+		foreach ($groupList as $group) {
+			if (in_array($group, $userGroups)) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	public function isEnabledForUser(string $userId = null): bool {
+		if ($this->userMatchesGroupList($userId, $this->config->getUseGroups())) {
+			return true;
+		}
+
+		return false;
+	}
+
+	public function userCanEdit(string $userId = null): bool {
+		if ($this->userMatchesGroupList($userId, $this->config->getEditGroups())) {
+			return true;
+		}
+
+		return false;
+	}
+
+	public function userIsFeatureLocked(string $userId = null): bool {
+		if ($this->config->isReadOnlyFeatureLocked() && !$this->userCanEdit($userId)) {
+			return true;
+		}
+
 		return false;
 	}
 }
--- a/tests/lib/PermissionManagerTest.php
+++ b/tests/lib/PermissionManagerTest.php
@ -21,12 +21,13 @@

 namespace Tests\Richdocuments;

+use OCA\Richdocuments\AppConfig;
 use OCA\Richdocuments\PermissionManager;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IUser;
+use OCP\IUserManager;
 use OCP\IUserSession;
-use PHPUnit\Framework\MockObject\MockBuilder;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;

@ -35,6 +36,8 @@ class PermissionManagerTest extends TestCase {
 	private $config;
 	/** @var IGroupManager|MockObject */
 	private $groupManager;
+	/** @var IUserManager|MockObject */
+	private $userManager;
 	/** @var IUserSession|MockObject */
 	private $userSession;
 	/** @var PermissionManager */
@ -42,91 +45,90 @@ class PermissionManagerTest extends TestCase {

 	public function setUp(): void {
 		parent::setUp();
-		$this->config = $this->createMock(IConfig::class);
+		$this->config = $this->createMock(AppConfig::class);
 		$this->groupManager = $this->createMock(IGroupManager::class);
+		$this->userManager = $this->createMock(IUserManager::class);
 		$this->userSession = $this->createMock(IUserSession::class);
-		$this->permissionManager = new PermissionManager($this->config, $this->groupManager, $this->userSession);
+		$this->permissionManager = new PermissionManager($this->config, $this->groupManager, $this->userManager, $this->userSession);
 	}

-	public function testIsEnabledForUserEnabledNoRestrictions() {
+	public function testIsEnabledForUserEnabledNoRestrictions(): void {
 		$this->config
 			->expects($this->once())
-			->method('getAppValue')
-			->with('richdocuments', 'use_groups', '')
-			->willReturn('');
+			->method('getUseGroups')
+			->willReturn(null);

 		$this->assertTrue($this->permissionManager->isEnabledForUser('TestUser'));
 	}

-	public function testIsEnabledForUserEnabledNotInGroupSession() {
-		/** @var IUser|MockBuilder $user */
-		$user = $this->createMock(IUser::class);
-		$user
-			->expects($this->any())
-			->method('getUID')
-			->willReturn('TestUser');
-		$this->userSession->expects($this->once())
-			->method('getUser')
-			->willReturn($user);
-
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('richdocuments', 'use_groups', '')
-			->willReturn('Enabled1|Enabled2|Enabled3');
-
-		$this->groupManager
-			->expects($this->at(0))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled1')
-			->willReturn(true);
-		$this->assertTrue($this->permissionManager->isEnabledForUser());
+	public function dataGroupMatchGroups(): array {
+		return [
+			[['admin', 'guests'], ['admin'], true],
+			[['admin', 'guests'], [], false],
+			[['group1', 'group2', 'group3'], [], false],
+			[['group1', 'group2', 'group3'], ['group1'], true],
+			[['group1', 'group2', 'group3'], ['group2'], true],
+			[['group1', 'group2', 'group3'], ['group0', 'group3'], true],
+			[['group1', 'group2', 'group3'], ['group1', 'group2'], true],
+			[[], [], true],
+		];
 	}

-	public function testIsEnabledForUserEnabledNotInGroup() {
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('richdocuments', 'use_groups', '')
-			->willReturn('Enabled1|Enabled2|Enabled3');
+	/** @dataProvider dataGroupMatchGroups */
+	public function testEditGroups($editGroups, $userGroups, $result): void {
+		$userMock = $this->createMock(IUser::class);
+		$this->config->expects($this->any())
+			->method('getEditGroups')
+			->willReturn($editGroups);
+		$this->userManager->expects($this->any())
+			->method('get')
+			->willReturn($userMock);
+		$this->groupManager->expects($this->any())
+			->method('getUserGroupIds')
+			->willReturn($userGroups);

-		$this->groupManager
-			->expects($this->at(0))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled1')
-			->willReturn(false);
-		$this->groupManager
-			->expects($this->at(1))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled2')
-			->willReturn(false);
-		$this->groupManager
-			->expects($this->at(2))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled3')
-			->willReturn(false);
-
-		$this->assertFalse($this->permissionManager->isEnabledForUser('TestUser'));
+		$this->assertEquals($result, $this->permissionManager->userCanEdit('user1'));
 	}

-	public function testIsEnabledForUserEnabledInGroup() {
-		$this->config
-			->expects($this->once())
-			->method('getAppValue')
-			->with('richdocuments', 'use_groups', '')
-			->willReturn('Enabled1|Enabled2|Enabled3');
+	/** @dataProvider dataGroupMatchGroups */
+	public function testUseGroups($editGroups, $userGroups, $result): void {
+		$userMock = $this->createMock(IUser::class);
+		$this->config->expects($this->any())
+			->method('getUseGroups')
+			->willReturn($editGroups);
+		$this->userManager->expects($this->any())
+			->method('get')
+			->willReturn($userMock);
+		$this->groupManager->expects($this->any())
+			->method('getUserGroupIds')
+			->willReturn($userGroups);

-		$this->groupManager
-			->expects($this->at(0))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled1')
-			->willReturn(false);
-		$this->groupManager
-			->expects($this->at(1))
-			->method('isInGroup')
-			->with('TestUser', 'Enabled2')
+		$this->assertEquals($result, $this->permissionManager->isEnabledForUser('user1'));
+	}
+
+	/** @dataProvider dataGroupMatchGroups */
+	public function testFeatureLock($editGroups, $userGroups, $result): void {
+		$userMock = $this->createMock(IUser::class);
+		$this->config->expects($this->any())
+			->method('getEditGroups')
+			->willReturn($editGroups);
+		$this->config->expects($this->any())
+			->method('isReadOnlyFeatureLocked')
 			->willReturn(true);
+		$this->userManager->expects($this->any())
+			->method('get')
+			->willReturn($userMock);
+		$this->groupManager->expects($this->any())
+			->method('getUserGroupIds')
+			->willReturn($userGroups);

-		$this->assertTrue($this->permissionManager->isEnabledForUser('TestUser'));
+		$canEdit = $this->permissionManager->userCanEdit('user1');
+		$isLocked = $this->permissionManager->userIsFeatureLocked('user1');
+
+		$this->assertEquals(!$result, $isLocked);
+		$this->assertEquals($result, $canEdit);
+
+		// Users with edit permission should never be locked
+		$this->assertFalse($isLocked && $canEdit);
 	}
 }