Skip to content

Commit

Permalink
Merge pull request #3116 from nextcloud/backport/3096/stable30
Browse files Browse the repository at this point in the history
[stable30] emit audit log events for changes to groupfolders
  • Loading branch information
icewind1991 authored Aug 15, 2024
2 parents fbf72d4 + 3b6a6ec commit 4f03530
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 51 deletions.
24 changes: 24 additions & 0 deletions lib/ACL/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
}
27 changes: 4 additions & 23 deletions lib/Command/ACL.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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('<error>incorrect format for permissions2 "' . $permission . '"</error>');
return -3;
}
Expand Down Expand Up @@ -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 [
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion lib/Controller/FolderController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}

Expand Down
32 changes: 23 additions & 9 deletions lib/DAV/ACLPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
38 changes: 25 additions & 13 deletions lib/Folder/FolderManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,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;
Expand All @@ -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,
) {
}

Expand Down Expand Up @@ -659,20 +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]));

/**
* @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;
}

/**
Expand All @@ -694,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]));
}

/**
Expand All @@ -715,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]));
}


Expand All @@ -739,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]));
}

/**
Expand All @@ -760,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]));
}

/**
Expand All @@ -771,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]));
}

/**
Expand All @@ -783,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]));
}

/**
Expand All @@ -795,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]));
}

/**
Expand Down Expand Up @@ -855,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]));
}

/**
Expand Down
14 changes: 9 additions & 5 deletions tests/Folder/FolderManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,18 +24,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();
}
Expand Down Expand Up @@ -86,7 +90,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' => []],
Expand Down Expand Up @@ -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();

Expand All @@ -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();

Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 4f03530

Please sign in to comment.