Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable30] emit audit log events for changes to groupfolders #3116

Merged
merged 3 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading