Merge pull request #3115 from nextcloud/backport/3096/stable29

[stable29] emit audit log events for changes to groupfolders

Author: icewind1991

lib/ACL/Rule.php

@@ -25,6 +25,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;
@@ -38,6 +39,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;
 
@@ -182,4 +191,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);
+	}
 }

lib/Command/ACL.php

@@ -39,14 +39,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;
@@ -103,7 +95,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 {
@@ -177,7 +169,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('<error>incorrect format for permissions2 "' . $permission . '"</error>');
 					return -3;
 				}
@@ -223,7 +215,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 [
@@ -244,23 +236,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;

lib/Controller/FolderController.php

@@ -185,7 +185,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]);
 	}
 

lib/DAV/ACLPlugin.php

@@ -29,8 +29,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;
@@ -46,19 +48,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 {
@@ -211,6 +208,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) {

lib/Folder/FolderManager.php

@@ -32,13 +32,15 @@
 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;
 use OCP\IDBConnection;
 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;
@@ -51,7 +53,8 @@ public function __construct(
 		private IDBConnection $connection,
 		private IGroupManager $groupManager,
 		private IMimeTypeLoader $mimeTypeLoader,
-		private LoggerInterface $logger
+		private LoggerInterface $logger,
+		private IEventDispatcher $eventDispatcher,
 	) {
 	}
 
@@ -674,20 +677,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]));
 
-	/**
-	 * @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();
+		return $id;
 	}
 
 	/**
@@ -709,6 +703,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]));
 	}
 
 	/**
@@ -730,6 +726,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]));
 	}
 
 
@@ -754,6 +752,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]));
 	}
 
 	/**
@@ -775,6 +775,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]));
 	}
 
 	/**
@@ -786,6 +789,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]));
 	}
 
 	/**
@@ -798,6 +803,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]));
 	}
 
 	/**
@@ -810,6 +817,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]));
 	}
 
 	/**
@@ -870,6 +879,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]));
 	}
 
 	/**

tests/Folder/FolderManagerTest.php

@@ -23,6 +23,7 @@
 
 use OCA\GroupFolders\Folder\FolderManager;
 use OCP\Constants;
+use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\IMimeTypeLoader;
 use OCP\IDBConnection;
 use OCP\IGroupManager;
@@ -38,18 +39,21 @@ class FolderManagerTest extends TestCase {
 	private IGroupManager $groupManager;
 	private IMimeTypeLoader $mimeLoader;
 	private LoggerInterface $logger;
+	private IEventDispatcher $eventDispatcher;
 
 	protected function setUp(): void {
 		parent::setUp();
 
 		$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();
 	}
@@ -101,7 +105,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' => []],
@@ -319,7 +323,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();
 
@@ -342,7 +346,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();
 
@@ -378,7 +382,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();
 

tests/stub.phpstub

@@ -1545,6 +1545,9 @@ namespace OC\Files\Storage\Wrapper{
 		public function getDirectoryContent($directory) : \Traversable
 		{
 		}
+		public function setOwner(?string $user) : void
+		{
+		}
 	}
 
 
@@ -1655,4 +1658,4 @@ namespace OCA\DAV\Connector\Sabre\Exception {
 		*/
 		public function serialize(\Sabre\DAV\Server $server, \DOMElement $errorNode) {}
 	}
-}
+}