Skip to content

Commit

Permalink
Merge pull request #2561 from nextcloud/backport/2560/stable27
Browse files Browse the repository at this point in the history
[stable27] pass around preload rules in acl wrapper instead of relying on the cache
  • Loading branch information
icewind1991 authored Sep 19, 2023
2 parents 6008669 + 7cc2c44 commit 6deffd9
Show file tree
Hide file tree
Showing 9 changed files with 80 additions and 30 deletions.
27 changes: 17 additions & 10 deletions lib/ACL/ACLCacheWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,15 @@
use OCP\Files\Search\ISearchQuery;

class ACLCacheWrapper extends CacheWrapper {
private $aclManager;
private $inShare;
private ACLManager $aclManager;
private bool $inShare;

private function getACLPermissionsForPath(string $path) {
$permissions = $this->aclManager->getACLPermissionsForPath($path);
private function getACLPermissionsForPath(string $path, array $rules = []) {
if ($rules) {
$permissions = $this->aclManager->getPermissionsForPathFromRules($path, $rules);
} else {
$permissions = $this->aclManager->getACLPermissionsForPath($path);
}

// if there is no read permissions, than deny everything
if ($this->inShare) {
Expand All @@ -52,10 +56,10 @@ public function __construct(ICache $cache, ACLManager $aclManager, bool $inShare
$this->inShare = $inShare;
}

protected function formatCacheEntry($entry) {
protected function formatCacheEntry($entry, array $rules = []) {
if (isset($entry['permissions'])) {
$entry['scan_permissions'] = $entry['permissions'];
$entry['permissions'] &= $this->getACLPermissionsForPath($entry['path']);
$entry['permissions'] &= $this->getACLPermissionsForPath($entry['path'], $rules);
if (!$entry['permissions']) {
return false;
}
Expand All @@ -65,8 +69,10 @@ protected function formatCacheEntry($entry) {

public function getFolderContentsById($fileId) {
$results = $this->getCache()->getFolderContentsById($fileId);
$this->preloadEntries($results);
$entries = array_map([$this, 'formatCacheEntry'], $results);
$rules = $this->preloadEntries($results);
$entries = array_map(function ($entry) use ($rules) {
return $this->formatCacheEntry($entry, $rules);
}, $results);
return array_filter(array_filter($entries));
}

Expand All @@ -90,11 +96,12 @@ public function searchQuery(ISearchQuery $query) {

/**
* @param ICacheEntry[] $entries
* @return Rule[][]
*/
private function preloadEntries(array $entries) {
private function preloadEntries(array $entries): array {
$paths = array_map(function (ICacheEntry $entry) {
return $entry->getPath();
}, $entries);
$this->aclManager->preloadPaths($paths);
return $this->aclManager->getRelevantRulesForPath($paths, false);
}
}
56 changes: 47 additions & 9 deletions lib/ACL/ACLManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ private function getRootStorageId(): int {
}

/**
* @param array $paths
* @return (Rule[])[]
* Get the list of rules applicable for a set of paths
*
* @param string[] $paths
* @param bool $cache whether to cache the retrieved rules
* @return array<string, Rule[]> sorted parent first
*/
private function getRules(array $paths): array {
private function getRules(array $paths, bool $cache = true): array {
// beware: adding new rules to the cache besides the cap
// might discard former cached entries, so we can't assume they'll stay
// cached, so we read everything out initially to be able to return it
Expand All @@ -74,7 +77,9 @@ private function getRules(array $paths): array {
if (!empty($nonCachedPaths)) {
$newRules = $this->ruleManager->getRulesForFilesByPath($this->user, $this->getRootStorageId(), $nonCachedPaths);
foreach ($newRules as $path => $rulesForPath) {
$this->ruleCache->set($path, $rulesForPath);
if ($cache) {
$this->ruleCache->set($path, $rulesForPath);
}
$rules[$path] = $rulesForPath;
}
}
Expand All @@ -85,10 +90,14 @@ private function getRules(array $paths): array {
}

/**
* Get a list of all path that might contain relevant rules when calculating the permissions for a path
*
* This contains the $path itself and any parent folder
*
* @param string $path
* @return string[]
*/
private function getParents(string $path): array {
private function getRelevantPaths(string $path): array {
$paths = [$path];
while ($path !== '') {
$path = dirname($path);
Expand All @@ -101,18 +110,47 @@ private function getParents(string $path): array {
return $paths;
}

public function preloadPaths(array $paths): void {
/**
* Get the list of rules applicable for a set of paths, including rules for any parent
*
* @param string[] $paths
* @param bool $cache whether to cache the retrieved rules
* @return array<string, Rule[]> sorted parent first
*/
public function getRelevantRulesForPath(array $paths, bool $cache = true): array {
$allPaths = [];
foreach ($paths as $path) {
$allPaths = array_unique(array_merge($allPaths, $this->getParents($path)));
$allPaths = array_unique(array_merge($allPaths, $this->getRelevantPaths($path)));
}
$this->getRules($allPaths);
return $this->getRules($allPaths, $cache);
}

public function getACLPermissionsForPath(string $path): int {
$path = ltrim($path, '/');
$rules = $this->getRules($this->getParents($path));
$rules = $this->getRules($this->getRelevantPaths($path));

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

/**
* @param string $path
* @param array<string, Rule[]> $rules list of rules per path
* @return int
*/
public function getPermissionsForPathFromRules(string $path, array $rules): int {
$path = ltrim($path, '/');
$relevantPaths = $this->getRelevantPaths($path);
$rules = array_intersect_key($rules, array_flip($relevantPaths));
return $this->calculatePermissionsForPath($rules);
}

/**
* @param array<string, Rule[]> $rules list of rules per path, sorted parent first
* @return int
*/
private function calculatePermissionsForPath(array $rules): int {
// first combine all rules with the same path, then apply them on top of the current permissions
// since $rules is sorted parent first rules for subfolders overwrite the rules from the parent
return array_reduce($rules, function (int $permissions, array $rules): int {
$mergedRule = Rule::mergeRules($rules);
return $mergedRule->applyPermissions($permissions);
Expand Down
8 changes: 8 additions & 0 deletions lib/ACL/Rule.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ public function getPermissions(): int {
return $this->permissions;
}

/**
* Apply this rule to an existing permission set, returning the resulting permissions
*
* All permissions included in the current mask will overwrite the existing permissions
*
* @param int $permissions
* @return int
*/
public function applyPermissions(int $permissions): int {
$invertedMask = ~$this->mask;
// create a bitmask that has all inherit and allow bits set to 1 and all deny bits to 0
Expand Down
1 change: 0 additions & 1 deletion lib/Listeners/CircleDestroyedEventListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
use OCP\EventDispatcher\IEventListener;

class CircleDestroyedEventListener implements IEventListener {

public function __construct(
private FolderManager $folderManager,
) {
Expand Down
1 change: 0 additions & 1 deletion lib/Migration/Version1401000Date20230426112001.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
use OCP\Migration\SimpleMigrationStep;

class Version1401000Date20230426112001 extends SimpleMigrationStep {

public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
/** @var ISchemaWrapper $schema */
$schema = $schemaClosure();
Expand Down
1 change: 0 additions & 1 deletion lib/Migration/Version16000Date20230821085801.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
* Auto-generated migration step: Please modify to your needs!
*/
class Version16000Date20230821085801 extends SimpleMigrationStep {

/**
* @param IOutput $output
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
Expand Down
14 changes: 8 additions & 6 deletions lib/Mount/MountProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
return $this->getJailPath($folder['folder_id']);
}, $foldersWithAcl);
$aclManager = $this->aclManagerFactory->getACLManager($user, $this->getRootStorageId());
$aclManager->preloadPaths($aclRootPaths);
$rootRules = $aclManager->getRelevantRulesForPath($aclRootPaths);

return array_values(array_filter(array_map(function ($folder) use ($user, $loader, $conflicts, $aclManager) {
return array_values(array_filter(array_map(function ($folder) use ($user, $loader, $conflicts, $aclManager, $rootRules) {
// check for existing files in the user home and rename them if needed
$originalFolderName = $folder['mount_point'];
if (in_array($originalFolderName, $conflicts)) {
Expand Down Expand Up @@ -168,7 +168,8 @@ public function getMountsForUser(IUser $user, IStorageFactory $loader) {
$loader,
$folder['acl'],
$user,
$aclManager
$aclManager,
$rootRules
);
}, $folders)));
}
Expand Down Expand Up @@ -200,7 +201,8 @@ public function getMount(
IStorageFactory $loader = null,
bool $acl = false,
IUser $user = null,
?ACLManager $aclManager = null
?ACLManager $aclManager = null,
array $rootRules = []
): ?IMountPoint {
if (!$cacheEntry) {
// trigger folder creation
Expand All @@ -219,12 +221,12 @@ public function getMount(
if ($acl && $user) {
$inShare = $this->getCurrentUID() === null || $this->getCurrentUID() !== $user->getUID();
$aclManager ??= $this->aclManagerFactory->getACLManager($user, $this->getRootStorageId());
$aclRootPermissions = $aclManager->getPermissionsForPathFromRules($rootPath, $rootRules);
$storage = new ACLStorageWrapper([
'storage' => $storage,
'acl_manager' => $aclManager,
'in_share' => $inShare
'in_share' => $inShare,
]);
$aclRootPermissions = $aclManager->getACLPermissionsForPath($rootPath);
$cacheEntry['permissions'] &= $aclRootPermissions;
}

Expand Down
1 change: 0 additions & 1 deletion lib/Settings/Admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
use OCP\AppFramework\Services\IInitialState;

class Admin implements IDelegatedSettings {

public function __construct(
private IInitialState $initialState,
private ApplicationService $applicationService,
Expand Down
1 change: 0 additions & 1 deletion lib/Versions/GroupVersion.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
use OCP\IUser;

class GroupVersion extends Version {

public function __construct(
int $timestamp,
int $revisionId,
Expand Down

0 comments on commit 6deffd9

Please sign in to comment.