Skip to content

Commit

Permalink
add check to prevent users from revoking their own access
Browse files Browse the repository at this point in the history
Signed-off-by: Robin Appelman <[email protected]>
  • Loading branch information
icewind1991 committed May 14, 2024
1 parent 7e15fb0 commit f38c222
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 17 deletions.
53 changes: 46 additions & 7 deletions lib/ACL/ACLManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
namespace OCA\GroupFolders\ACL;

use OC\Cache\CappedMemoryCache;
use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
use OCA\GroupFolders\Trash\TrashManager;
use OCP\Constants;
use OCP\Files\IRootFolder;
Expand All @@ -35,12 +36,13 @@ class ACLManager {
private $rootFolderProvider;

public function __construct(
private RuleManager $ruleManager,
private TrashManager $trashManager,
private IUser $user,
callable $rootFolderProvider,
private ?int $rootStorageId = null,
private bool $inheritMergePerUser = false,
private RuleManager $ruleManager,
private TrashManager $trashManager,
private IUserMappingManager $userMappingManager,
private IUser $user,
callable $rootFolderProvider,
private ?int $rootStorageId = null,
private bool $inheritMergePerUser = false,
) {
$this->ruleCache = new CappedMemoryCache();
$this->rootFolderProvider = $rootFolderProvider;
Expand Down Expand Up @@ -104,7 +106,7 @@ private function getRelevantPaths(string $path): array {
$fromTrashbin = str_starts_with($path, '__groupfolders/trash/');
if ($fromTrashbin) {
/* Exploded path will look like ["__groupfolders", "trash", "1", "folderName.d2345678", "rest/of/the/path.txt"] */
[,,$groupFolderId,$rootTrashedItemName] = explode('/', $path, 5);
[, , $groupFolderId, $rootTrashedItemName] = explode('/', $path, 5);
$groupFolderId = (int)$groupFolderId;
/* Remove the date part */
$separatorPos = strrpos($rootTrashedItemName, '.d');
Expand Down Expand Up @@ -152,6 +154,22 @@ public function getACLPermissionsForPath(string $path): int {
return $this->calculatePermissionsForPath($rules);
}

/**
* Check what the effective permissions would be for the current user for a path would be with a new set of rules
*
* @param string $path
* @param array $newRules
* @return int
*/
public function testACLPermissionsForPath(string $path, array $newRules): int {
$path = ltrim($path, '/');
$rules = $this->getRules($this->getRelevantPaths($path));

$rules[$path] = $this->filterApplicableRulesToUser($newRules);

return $this->calculatePermissionsForPath($rules);
}

/**
* @param string $path
* @param array<string, Rule[]> $rules list of rules per path
Expand Down Expand Up @@ -235,4 +253,25 @@ public function getPermissionsForTree(string $path): int {
return $permissions & $denyMask;
}, Constants::PERMISSION_ALL);
}

/**
* Filter a list to only the rules applicable to the current user
*
* @param Rule[] $rules
* @return Rule[]
*/
private function filterApplicableRulesToUser(array $rules): array {
$userMappings = $this->userMappingManager->getMappingsForUser($this->user);
return array_values(array_filter($rules, function(Rule $rule) use ($userMappings) {
foreach ($userMappings as $userMapping) {
if (
$userMapping->getType() == $rule->getUserMapping()->getType() &&
$userMapping->getId() == $rule->getUserMapping()->getId()
) {
return true;
}
}
return false;
}));
}
}
3 changes: 3 additions & 0 deletions lib/ACL/ACLManagerFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

namespace OCA\GroupFolders\ACL;

use OCA\GroupFolders\ACL\UserMapping\IUserMappingManager;
use OCA\GroupFolders\Trash\TrashManager;
use OCP\IConfig;
use OCP\IUser;
Expand All @@ -34,6 +35,7 @@ public function __construct(
private RuleManager $ruleManager,
private TrashManager $trashManager,
private IConfig $config,
private IUserMappingManager $userMappingManager,
callable $rootFolderProvider,
) {
$this->rootFolderProvider = $rootFolderProvider;
Expand All @@ -43,6 +45,7 @@ public function getACLManager(IUser $user, ?int $rootStorageId = null): ACLManag
return new ACLManager(
$this->ruleManager,
$this->trashManager,
$this->userMappingManager,
$user,
$this->rootFolderProvider,
$rootStorageId,
Expand Down
1 change: 1 addition & 0 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ public function register(IRegistrationContext $context): void {
$c->get(RuleManager::class),
$c->get(TrashManager::class),
$c->get(IConfig::class),
$c->get(IUserMappingManager::class),
$rootFolderProvider
);
});
Expand Down
24 changes: 14 additions & 10 deletions lib/DAV/ACLPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@
namespace OCA\GroupFolders\DAV;

use OCA\DAV\Connector\Sabre\Node;
use OCA\GroupFolders\ACL\ACLManager;
use OCA\GroupFolders\ACL\ACLManagerFactory;
use OCA\GroupFolders\ACL\Rule;
use OCA\GroupFolders\ACL\RuleManager;
use OCA\GroupFolders\Folder\FolderManager;
use OCA\GroupFolders\Mount\GroupMountPoint;
use OCP\Constants;
use OCP\IUser;
use OCP\IUserSession;
use Sabre\DAV\Exception\BadRequest;
use Sabre\DAV\INode;
use Sabre\DAV\PropFind;
use Sabre\DAV\PropPatch;
Expand All @@ -46,19 +49,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 ACLManagerFactory $aclManagerFactory,
) {
$this->ruleManager = $ruleManager;
$this->userSession = $userSession;
$this->folderManager = $folderManager;
}

private function isAdmin(string $path): bool {
Expand All @@ -72,7 +70,7 @@ private function isAdmin(string $path): bool {

public function initialize(Server $server): void {
$this->server = $server;
$this->user = $user = $this->userSession->getUser();
$this->user = $this->userSession->getUser();

$this->server->on('propFind', [$this, 'propFind']);
$this->server->on('propPatch', [$this, 'propPatch']);
Expand Down Expand Up @@ -211,6 +209,12 @@ public function propPatch(string $path, PropPatch $propPatch): void {
);
}, $rawRules);

$aclManager = $this->aclManagerFactory->getACLManager($this->user);
$newPermissions = $aclManager->testACLPermissionsForPath($fileInfo->getPath(), $rules);
if (!($newPermissions & Constants::PERMISSION_READ)) {
throw new BadRequest("Request would revoke permissions for current user");
}

$existingRules = array_reduce(
$this->ruleManager->getAllRulesForPaths($mount->getNumericStorageId(), [$path]),
function (array $rules, array $rulesForPath) {
Expand Down

0 comments on commit f38c222

Please sign in to comment.