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

[stable27] pass around preload rules in acl wrapper instead of relying on the cache #2561

Merged
merged 3 commits into from
Sep 19, 2023
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
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
Loading