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

feat: Save deleted by property #3170

Merged
merged 1 commit into from
Sep 9, 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
2 changes: 1 addition & 1 deletion appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Folders can be configured from *Group folders* in the admin settings.

After a folder is created, the admin can give access to the folder to one or more groups, control their write/sharing permissions and assign a quota for the folder.
]]></description>
<version>19.0.0-dev.2</version>
<version>19.0.0-dev.3</version>
<licence>agpl</licence>
<author>Robin Appelman</author>
<namespace>GroupFolders</namespace>
Expand Down
5 changes: 4 additions & 1 deletion lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
use OCP\IGroupManager;
use OCP\IRequest;
use OCP\ISession;
use OCP\IUserManager;
use OCP\IUserSession;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -126,7 +127,9 @@ public function register(IRegistrationContext $context): void {
$c->get(MountProvider::class),
$c->get(ACLManagerFactory::class),
$c->get(IRootFolder::class),
$c->get(LoggerInterface::class)
$c->get(LoggerInterface::class),
$c->get(IUserManager::class),
$c->get(IUserSession::class),
);
$hasVersionApp = interface_exists(\OCA\Files_Versions\Versions\IVersionBackend::class);
if ($hasVersionApp) {
Expand Down
47 changes: 47 additions & 0 deletions lib/Migration/Version300000Date20240905185515.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\GroupFolders\Migration;

use Closure;
use OCP\DB\ISchemaWrapper;
use OCP\DB\Types;
use OCP\Migration\IOutput;
use OCP\Migration\SimpleMigrationStep;

/**
* Adds the delete_by column to the group_folders_trash table
*/
class Version300000Date20240905185515 extends SimpleMigrationStep {

/**
* @param Closure(): ISchemaWrapper $schemaClosure
*/
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();

if (!$schema->hasTable('group_folders_trash')) {
return null;
}

$table = $schema->getTable('group_folders_trash');

if ($table->hasColumn('deleted_by')) {
return null;
}

$table->addColumn('deleted_by', Types::STRING, [
'notnull' => false,
'length' => 64,
]);

return $schema;
}
}
5 changes: 3 additions & 2 deletions lib/Trash/GroupTrashItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ public function __construct(
string $trashPath,
FileInfo $fileInfo,
IUser $user,
string $mountPoint
string $mountPoint,
private ?IUser $deletedBy,
) {
parent::__construct($backend, $originalLocation, $deletedTime, $trashPath, $fileInfo, $user, null);
parent::__construct($backend, $originalLocation, $deletedTime, $trashPath, $fileInfo, $user, $deletedBy);
$this->mountPoint = $mountPoint;
}

Expand Down
41 changes: 17 additions & 24 deletions lib/Trash/TrashBackend.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,25 @@
use OCP\Files\NotPermittedException;
use OCP\Files\Storage\IStorage;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use Psr\Log\LoggerInterface;

class TrashBackend implements ITrashBackend {
private FolderManager $folderManager;
private TrashManager $trashManager;
private Folder $appFolder;
private MountProvider $mountProvider;
private ACLManagerFactory $aclManagerFactory;
/** @var ?VersionsBackend */
private $versionsBackend = null;
private IRootFolder $rootFolder;
private LoggerInterface $logger;

public function __construct(
FolderManager $folderManager,
TrashManager $trashManager,
Folder $appFolder,
MountProvider $mountProvider,
ACLManagerFactory $aclManagerFactory,
IRootFolder $rootFolder,
LoggerInterface $logger
private FolderManager $folderManager,
private TrashManager $trashManager,
private Folder $appFolder,
private MountProvider $mountProvider,
private ACLManagerFactory $aclManagerFactory,
private IRootFolder $rootFolder,
private LoggerInterface $logger,
private IUserManager $userManager,
private IUserSession $userSession,
) {
$this->folderManager = $folderManager;
$this->trashManager = $trashManager;
$this->appFolder = $appFolder;
$this->mountProvider = $mountProvider;
$this->aclManagerFactory = $aclManagerFactory;
$this->rootFolder = $rootFolder;
$this->logger = $logger;
}

public function setVersionsBackend(VersionsBackend $versionsBackend): void {
Expand Down Expand Up @@ -92,7 +82,8 @@ public function listTrashFolder(ITrashItem $folder): array {
$folder->getTrashPath() . '/' . $node->getName(),
$node,
$user,
$folder->getGroupFolderMountPoint()
$folder->getGroupFolderMountPoint(),
$folder->getDeletedBy(),
);
}, $content)));
}
Expand Down Expand Up @@ -212,7 +203,7 @@ public function moveToTrash(IStorage $storage, string $internalPath): bool {
[$unJailedStorage, $unJailedInternalPath] = $this->unwrapJails($storage, $internalPath);
$targetInternalPath = $trashFolder->getInternalPath() . '/' . $trashName;
if ($trashStorage->moveFromStorage($unJailedStorage, $unJailedInternalPath, $targetInternalPath)) {
$this->trashManager->addTrashItem($folderId, $name, $time, $internalPath, $fileEntry->getId());
$this->trashManager->addTrashItem($folderId, $name, $time, $internalPath, $fileEntry->getId(), $this->userSession->getUser()->getUID());
if ($trashStorage->getCache()->getId($targetInternalPath) !== $fileEntry->getId()) {
$trashStorage->getCache()->moveFromCache($unJailedStorage->getCache(), $unJailedInternalPath, $targetInternalPath);
}
Expand Down Expand Up @@ -328,6 +319,7 @@ private function getTrashForFolders(IUser $user, array $folders): array {
$key = $folderId . '/' . $name . '/' . $timestamp;

$originalLocation = isset($indexedRows[$key]) ? $indexedRows[$key]['original_location'] : '';
$deletedBy = isset($indexedRows[$key]) ? $indexedRows[$key]['deleted_by'] : '';

if ($folderHasAcl) {
// if we for any reason lost track of the original location, hide the item for non-managers as a fail-safe
Expand Down Expand Up @@ -357,7 +349,8 @@ private function getTrashForFolders(IUser $user, array $folders): array {
'/' . $folderId . '/' . $item->getName(),
$info,
$user,
$mountPoint
$mountPoint,
$this->userManager->get($deletedBy),
);
}
}
Expand Down
11 changes: 6 additions & 5 deletions lib/Trash/TrashManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,37 +22,38 @@ public function __construct(
*/
public function listTrashForFolders(array $folderIds): array {
$query = $this->connection->getQueryBuilder();
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id', 'file_id'])
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id', 'file_id', 'deleted_by'])
->from('group_folders_trash')
->orderBy('deleted_time')
->where($query->expr()->in('folder_id', $query->createNamedParameter($folderIds, IQueryBuilder::PARAM_INT_ARRAY)));
return $query->executeQuery()->fetchAll();
}

public function addTrashItem(int $folderId, string $name, int $deletedTime, string $originalLocation, int $fileId): void {
public function addTrashItem(int $folderId, string $name, int $deletedTime, string $originalLocation, int $fileId, string $deletedBy): void {
$query = $this->connection->getQueryBuilder();
$query->insert('group_folders_trash')
->values([
'folder_id' => $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT),
'name' => $query->createNamedParameter($name),
'deleted_time' => $query->createNamedParameter($deletedTime, IQueryBuilder::PARAM_INT),
'original_location' => $query->createNamedParameter($originalLocation),
'file_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)
'file_id' => $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT),
'deleted_by' => $query->createNamedParameter($deletedBy),
]);
$query->executeStatement();
}

public function getTrashItemByFileId(int $fileId): ?array {
$query = $this->connection->getQueryBuilder();
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id'])
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id', 'deleted_by'])
->from('group_folders_trash')
->where($query->expr()->eq('file_id', $query->createNamedParameter($fileId, IQueryBuilder::PARAM_INT)));
return $query->executeQuery()->fetch() ?: null;
}

public function getTrashItemByFileName(int $folderId, string $name, int $deletedTime): ?array {
$query = $this->connection->getQueryBuilder();
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id'])
$query->select(['trash_id', 'name', 'deleted_time', 'original_location', 'folder_id', 'deleted_by'])
->from('group_folders_trash')
->where($query->expr()->eq('folder_id', $query->createNamedParameter($folderId, IQueryBuilder::PARAM_INT)))
->andWhere($query->expr()->eq('name', $query->createNamedParameter($name)))
Expand Down
16 changes: 16 additions & 0 deletions tests/Trash/TrashBackendTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ private function createNoReadRule(string $userId, int $fileId): Rule {
}

public function testHideTrashItemAcl() {
$this->loginAsUser('manager');

$restricted = $this->managerUserFolder->newFile("{$this->folderName}/restricted.txt", "content");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $restricted->getId()));

Expand All @@ -120,9 +122,13 @@ public function testHideTrashItemAcl() {
// only the manager can see the deleted file
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));

$this->logout();
}

public function testHideItemInDeletedFolderAcl() {
$this->loginAsUser('manager');

$folder = $this->managerUserFolder->newFolder("{$this->folderName}/folder");
$folder->newFile("file.txt", "content1");
$restrictedChild = $folder->newFile("restricted.txt", "content2");
Expand All @@ -145,9 +151,13 @@ public function testHideItemInDeletedFolderAcl() {
// only the manager can see the restricted child, both can see the un-restricted child
$this->assertCount(2, $this->trashBackend->listTrashFolder($managerTrashFolder));
$this->assertCount(1, $this->trashBackend->listTrashFolder($normalTrashFolder));

$this->logout();
}

public function testHideDeletedTrashItemInDeletedFolderAcl() {
$this->loginAsUser('manager');

$folder = $this->managerUserFolder->newFolder("{$this->folderName}/restricted");
$child = $folder->newFile("file.txt", "content1");
$this->ruleManager->saveRule($this->createNoReadRule("normal", $folder->getId()));
Expand All @@ -169,9 +179,13 @@ public function testHideDeletedTrashItemInDeletedFolderAcl() {
// only the manager can see the deleted items
$this->assertCount(2, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(0, $this->trashBackend->listTrashRoot($this->normalUser));

$this->logout();
}

public function testHideDeletedTrashItemInDeletedParentFolderAcl() {
$this->loginAsUser('manager');

$parent = $this->managerUserFolder->newFolder("{$this->folderName}/parent");
$folder = $parent->newFolder("restricted");
$child = $folder->newFile("file.txt", "content1");
Expand All @@ -194,5 +208,7 @@ public function testHideDeletedTrashItemInDeletedParentFolderAcl() {
// only the manager can see the deleted child, both can see the deleted parent
$this->assertCount(2, $this->trashBackend->listTrashRoot($this->managerUser));
$this->assertCount(1, $this->trashBackend->listTrashRoot($this->normalUser));

$this->logout();
}
}
Loading