From 0b3485796f859411feb3d382787b386da37490d6 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 7 Aug 2024 12:34:22 +0200 Subject: [PATCH 1/3] chore: remove duplicate rename implementation Signed-off-by: Robin Appelman --- lib/Controller/FolderController.php | 2 +- lib/Folder/FolderManager.php | 12 ------------ tests/Folder/FolderManagerTest.php | 2 +- 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/lib/Controller/FolderController.php b/lib/Controller/FolderController.php index 57824ec4c..2578a458a 100644 --- a/lib/Controller/FolderController.php +++ b/lib/Controller/FolderController.php @@ -171,7 +171,7 @@ public function removeFolder(int $id): DataResponse { * @RequireGroupFolderAdmin */ public function setMountPoint(int $id, string $mountPoint): DataResponse { - $this->manager->setMountPoint($id, trim($mountPoint)); + $this->manager->renameFolder($id, trim($mountPoint)); return new DataResponse(['success' => true]); } diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 9a6ceefbe..24bee259d 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -663,18 +663,6 @@ public function createFolder(string $mountPoint): int { return $query->getLastInsertId(); } - /** - * @throws Exception - */ - public function setMountPoint(int $folderId, string $mountPoint): void { - $query = $this->connection->getQueryBuilder(); - - $query->update('group_folders') - ->set('mount_point', $query->createNamedParameter($mountPoint)) - ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); - $query->executeStatement(); - } - /** * @throws Exception */ diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php index b2d71cde0..fc130640d 100644 --- a/tests/Folder/FolderManagerTest.php +++ b/tests/Folder/FolderManagerTest.php @@ -86,7 +86,7 @@ public function testSetMountpoint() { $folderId1 = $this->manager->createFolder('foo'); $this->manager->createFolder('bar'); - $this->manager->setMountPoint($folderId1, 'foo2'); + $this->manager->renameFolder($folderId1, 'foo2'); $this->assertHasFolders([ ['mount_point' => 'foo2', 'groups' => []], From e73f276e9c713266bc50f30d08e6b94d12ecb917 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 7 Aug 2024 13:11:16 +0200 Subject: [PATCH 2/3] chore: move rule permissions formatting logic out of command Signed-off-by: Robin Appelman --- lib/ACL/Rule.php | 24 ++++++++++++++++++++++++ lib/Command/ACL.php | 27 ++++----------------------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/lib/ACL/Rule.php b/lib/ACL/Rule.php index a3bbb8076..068fd409e 100644 --- a/lib/ACL/Rule.php +++ b/lib/ACL/Rule.php @@ -10,6 +10,7 @@ use OCA\GroupFolders\ACL\UserMapping\IUserMapping; use OCA\GroupFolders\ACL\UserMapping\UserMapping; +use OCP\Constants; use Sabre\Xml\Reader; use Sabre\Xml\Writer; use Sabre\Xml\XmlDeserializable; @@ -23,6 +24,14 @@ class Rule implements XmlSerializable, XmlDeserializable, \JsonSerializable { public const MAPPING_ID = '{http://nextcloud.org/ns}acl-mapping-id'; public const MAPPING_DISPLAY_NAME = '{http://nextcloud.org/ns}acl-mapping-display-name'; + public const PERMISSIONS_MAP = [ + 'read' => Constants::PERMISSION_READ, + 'write' => Constants::PERMISSION_UPDATE, + 'create' => Constants::PERMISSION_CREATE, + 'delete' => Constants::PERMISSION_DELETE, + 'share' => Constants::PERMISSION_SHARE, + ]; + private $userMapping; private $fileId; @@ -167,4 +176,19 @@ public static function defaultRule(): Rule { 0 ); } + + public static function formatRulePermissions(int $mask, int $permissions): string { + $result = []; + foreach (self::PERMISSIONS_MAP as $name => $value) { + if (($mask & $value) === $value) { + $type = ($permissions & $value) === $value ? '+' : '-'; + $result[] = $type . $name; + } + } + return implode(', ', $result); + } + + public function formatPermissions(): string { + return self::formatRulePermissions($this->mask, $this->permissions); + } } diff --git a/lib/Command/ACL.php b/lib/Command/ACL.php index cbf029cbf..f63859a8f 100644 --- a/lib/Command/ACL.php +++ b/lib/Command/ACL.php @@ -24,14 +24,6 @@ use Symfony\Component\Console\Output\OutputInterface; class ACL extends FolderCommand { - public const PERMISSIONS_MAP = [ - 'read' => Constants::PERMISSION_READ, - 'write' => Constants::PERMISSION_UPDATE, - 'create' => Constants::PERMISSION_CREATE, - 'delete' => Constants::PERMISSION_DELETE, - 'share' => Constants::PERMISSION_SHARE, - ]; - private RuleManager $ruleManager; private ACLManagerFactory $aclManagerFactory; private IUserManager $userManager; @@ -88,7 +80,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { $path = $input->getArgument('path'); $aclManager = $this->aclManagerFactory->getACLManager($user); $permissions = $aclManager->getACLPermissionsForPath($jailPath . rtrim('/' . $path, '/')); - $permissionString = $this->formatRulePermissions(Constants::PERMISSION_ALL, $permissions); + $permissionString = Rule::formatRulePermissions(Constants::PERMISSION_ALL, $permissions); $output->writeln($permissionString); return 0; } else { @@ -162,7 +154,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { return -3; } $name = substr($permission, 1); - if (!isset(self::PERMISSIONS_MAP[$name])) { + if (!isset(Rule::PERMISSIONS_MAP[$name])) { $output->writeln('incorrect format for permissions2 "' . $permission . '"'); return -3; } @@ -208,7 +200,7 @@ private function printPermissions(InputInterface $input, OutputInterface $output return $rule->getUserMapping()->getType() . ': ' . $rule->getUserMapping()->getId(); }, $rulesForPath); $permissions = array_map(function (Rule $rule) { - return $this->formatRulePermissions($rule->getMask(), $rule->getPermissions()); + return $rule->formatPermissions(); }, $rulesForPath); $formattedPath = substr($path, $jailPathLength); return [ @@ -229,23 +221,12 @@ private function printPermissions(InputInterface $input, OutputInterface $output } } - private function formatRulePermissions(int $mask, int $permissions): string { - $result = []; - foreach (self::PERMISSIONS_MAP as $name => $value) { - if (($mask & $value) === $value) { - $type = ($permissions & $value) === $value ? '+' : '-'; - $result[] = $type . $name; - } - } - return implode(', ', $result); - } - private function parsePermissions(array $permissions): array { $mask = 0; $result = 0; foreach ($permissions as $permission) { - $permissionValue = self::PERMISSIONS_MAP[substr($permission, 1)]; + $permissionValue = Rule::PERMISSIONS_MAP[substr($permission, 1)]; $mask |= $permissionValue; if ($permission[0] === '+') { $result |= $permissionValue; From 3b6a6ecff6ed39af5c538e696b991bcdbff9e3be Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Wed, 7 Aug 2024 13:31:30 +0200 Subject: [PATCH 3/3] feat: emit audit log events for changes to groupfolders Signed-off-by: Robin Appelman --- lib/DAV/ACLPlugin.php | 32 +++++++++++++++++++++--------- lib/Folder/FolderManager.php | 28 ++++++++++++++++++++++++-- tests/Folder/FolderManagerTest.php | 12 +++++++---- 3 files changed, 57 insertions(+), 15 deletions(-) diff --git a/lib/DAV/ACLPlugin.php b/lib/DAV/ACLPlugin.php index a660ae2ee..0ff64151f 100644 --- a/lib/DAV/ACLPlugin.php +++ b/lib/DAV/ACLPlugin.php @@ -14,8 +14,10 @@ use OCA\GroupFolders\Folder\FolderManager; use OCA\GroupFolders\Mount\GroupMountPoint; use OCP\Constants; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IUser; use OCP\IUserSession; +use OCP\Log\Audit\CriticalActionPerformedEvent; use Sabre\DAV\INode; use Sabre\DAV\PropFind; use Sabre\DAV\PropPatch; @@ -31,19 +33,14 @@ class ACLPlugin extends ServerPlugin { public const GROUP_FOLDER_ID = '{http://nextcloud.org/ns}group-folder-id'; private ?Server $server = null; - private RuleManager $ruleManager; - private FolderManager $folderManager; - private IUserSession $userSession; private ?IUser $user = null; public function __construct( - RuleManager $ruleManager, - IUserSession $userSession, - FolderManager $folderManager + private RuleManager $ruleManager, + private IUserSession $userSession, + private FolderManager $folderManager, + private IEventDispatcher $eventDispatcher, ) { - $this->ruleManager = $ruleManager; - $this->userSession = $userSession; - $this->folderManager = $folderManager; } private function isAdmin(string $path): bool { @@ -196,6 +193,23 @@ public function propPatch(string $path, PropPatch $propPatch): void { ); }, $rawRules); + $formattedRules = array_map(function (Rule $rule) { + return $rule->getUserMapping()->getType() . ' ' . $rule->getUserMapping()->getDisplayName() . ': ' . $rule->formatPermissions(); + }, $rules); + if (count($formattedRules)) { + $formattedRules = implode(', ', $formattedRules); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The advanced permissions for "%s" in groupfolder with id %d was set to "%s"', [ + $fileInfo->getInternalPath(), + $mount->getFolderId(), + $formattedRules, + ])); + } else { + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The advanced permissions for "%s" in groupfolder with id %d was cleared', [ + $fileInfo->getInternalPath(), + $mount->getFolderId(), + ])); + } + $existingRules = array_reduce( $this->ruleManager->getAllRulesForPaths($mount->getNumericStorageId(), [$path]), function (array $rules, array $rulesForPath) { diff --git a/lib/Folder/FolderManager.php b/lib/Folder/FolderManager.php index 24bee259d..f51cf9161 100644 --- a/lib/Folder/FolderManager.php +++ b/lib/Folder/FolderManager.php @@ -17,6 +17,7 @@ use OCP\Constants; use OCP\DB\Exception; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\Cache\ICacheEntry; use OCP\Files\IMimeTypeLoader; use OCP\Files\IRootFolder; @@ -24,6 +25,7 @@ use OCP\IGroupManager; use OCP\IUser; use OCP\IUserManager; +use OCP\Log\Audit\CriticalActionPerformedEvent; use OCP\Server; use Psr\Container\ContainerExceptionInterface; use Psr\Log\LoggerInterface; @@ -36,7 +38,8 @@ public function __construct( private IDBConnection $connection, private IGroupManager $groupManager, private IMimeTypeLoader $mimeTypeLoader, - private LoggerInterface $logger + private LoggerInterface $logger, + private IEventDispatcher $eventDispatcher, ) { } @@ -659,8 +662,11 @@ public function createFolder(string $mountPoint): int { 'mount_point' => $query->createNamedParameter($mountPoint) ]); $query->executeStatement(); + $id = $query->getLastInsertId(); - return $query->getLastInsertId(); + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('A new groupfolder "%s" was created with id %d', [$mountPoint, $id])); + + return $id; } /** @@ -682,6 +688,8 @@ public function addApplicableGroup(int $folderId, string $groupId): void { 'permissions' => $query->createNamedParameter(Constants::PERMISSION_ALL) ]); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The group "%s" was given access to the groupfolder with id %d', [$groupId, $folderId])); } /** @@ -703,6 +711,8 @@ public function removeApplicableGroup(int $folderId, string $groupId): void { ) ); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The group "%s" was revoked access to the groupfolder with id %d', [$groupId, $folderId])); } @@ -727,6 +737,8 @@ public function setGroupPermissions(int $folderId, string $groupId, int $permiss ); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The permissions of group "%s" to the groupfolder with id %d was set to %d', [$groupId, $folderId, $permissions])); } /** @@ -748,6 +760,9 @@ public function setManageACL(int $folderId, string $type, string $id, bool $mana ->andWhere($query->expr()->eq('mapping_id', $query->createNamedParameter($id))); } $query->executeStatement(); + + $action = $manageAcl ? "given" : "revoked"; + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The %s "%s" was %s acl management rights to the groupfolder with id %d', [$type, $id, $action, $folderId])); } /** @@ -759,6 +774,8 @@ public function removeFolder(int $folderId): void { $query->delete('group_folders') ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The groupfolder with id %d was removed', [$folderId])); } /** @@ -771,6 +788,8 @@ public function setFolderQuota(int $folderId, int $quota): void { ->set('quota', $query->createNamedParameter($quota)) ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId))); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The quota for groupfolder with id %d was set to %d bytes', [$folderId, $quota])); } /** @@ -783,6 +802,8 @@ public function renameFolder(int $folderId, string $newMountPoint): void { ->set('mount_point', $query->createNamedParameter($newMountPoint)) ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT))); $query->executeStatement(); + + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('The groupfolder with id %d was renamed to "%s"', [$folderId, $newMountPoint])); } /** @@ -843,6 +864,9 @@ public function setFolderACL(int $folderId, bool $acl): void { ->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId))); $query->executeStatement(); } + + $action = $acl ? "enabled" : "disabled"; + $this->eventDispatcher->dispatchTyped(new CriticalActionPerformedEvent('Advanced permissions for the groupfolder with id %d was %s', [$folderId, $action])); } /** diff --git a/tests/Folder/FolderManagerTest.php b/tests/Folder/FolderManagerTest.php index fc130640d..f09563805 100644 --- a/tests/Folder/FolderManagerTest.php +++ b/tests/Folder/FolderManagerTest.php @@ -8,6 +8,7 @@ use OCA\GroupFolders\Folder\FolderManager; use OCP\Constants; +use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IMimeTypeLoader; use OCP\IDBConnection; use OCP\IGroupManager; @@ -23,6 +24,7 @@ class FolderManagerTest extends TestCase { private IGroupManager $groupManager; private IMimeTypeLoader $mimeLoader; private LoggerInterface $logger; + private IEventDispatcher $eventDispatcher; protected function setUp(): void { parent::setUp(); @@ -30,11 +32,13 @@ protected function setUp(): void { $this->groupManager = $this->createMock(IGroupManager::class); $this->mimeLoader = $this->createMock(IMimeTypeLoader::class); $this->logger = $this->createMock(LoggerInterface::class); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); $this->manager = new FolderManager( \OC::$server->getDatabaseConnection(), $this->groupManager, $this->mimeLoader, - $this->logger + $this->logger, + $this->eventDispatcher, ); $this->clean(); } @@ -304,7 +308,7 @@ public function testGetFoldersForUserSimple() { $db = $this->createMock(IDBConnection::class); /** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMockBuilder(FolderManager::class) - ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger]) + ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher]) ->setMethods(['getFoldersForGroups']) ->getMock(); @@ -327,7 +331,7 @@ public function testGetFoldersForUserMerge() { $db = $this->createMock(IDBConnection::class); /** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMockBuilder(FolderManager::class) - ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger]) + ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher]) ->setMethods(['getFoldersForGroups']) ->getMock(); @@ -363,7 +367,7 @@ public function testGetFolderPermissionsForUserMerge() { $db = $this->createMock(IDBConnection::class); /** @var FolderManager|\PHPUnit_Framework_MockObject_MockObject $manager */ $manager = $this->getMockBuilder(FolderManager::class) - ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger]) + ->setConstructorArgs([$db, $this->groupManager, $this->mimeLoader, $this->logger, $this->eventDispatcher]) ->setMethods(['getFoldersForGroups']) ->getMock();