Skip to content

Commit 98ce44b

Browse files
committed
fix: filter ACL forbidden paths from search query
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent ec77268 commit 98ce44b

3 files changed

Lines changed: 63 additions & 8 deletions

File tree

β€Žlib/ACL/ACLCacheWrapper.phpβ€Ž

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,14 @@
99
namespace OCA\GroupFolders\ACL;
1010

1111
use OC\Files\Cache\Wrapper\CacheWrapper;
12+
use OC\Files\Search\SearchBinaryOperator;
13+
use OC\Files\Search\SearchComparison;
1214
use OCP\Constants;
1315
use OCP\Files\Cache\ICache;
1416
use OCP\Files\Cache\ICacheEntry;
17+
use OCP\Files\Search\ISearchBinaryOperator;
18+
use OCP\Files\Search\ISearchComparison;
19+
use OCP\Files\Search\ISearchOperator;
1520
use OCP\Files\Search\ISearchQuery;
1621

1722
class ACLCacheWrapper extends CacheWrapper {
@@ -75,23 +80,23 @@ public function getFolderContentsById($fileId, ?string $mimeTypeFilter = null):
7580

7681
#[\Override]
7782
public function search($pattern): array {
78-
$results = $this->getCache()->search($pattern);
83+
$results = parent::search($pattern);
7984
$this->preloadEntries($results);
8085

8186
return array_filter(array_map($this->formatCacheEntry(...), $results));
8287
}
8388

8489
#[\Override]
8590
public function searchByMime($mimetype): array {
86-
$results = $this->getCache()->searchByMime($mimetype);
91+
$results = parent::searchByMime($mimetype);
8792
$this->preloadEntries($results);
8893

8994
return array_filter(array_map($this->formatCacheEntry(...), $results));
9095
}
9196

9297
#[\Override]
9398
public function searchQuery(ISearchQuery $query): array {
94-
$results = $this->getCache()->searchQuery($query);
99+
$results = parent::searchQuery($query);
95100
$this->preloadEntries($results);
96101

97102
return array_filter(array_map($this->formatCacheEntry(...), $results));
@@ -106,4 +111,32 @@ private function preloadEntries(array $entries): array {
106111

107112
return $this->aclManager->getRelevantRulesForPath($this->getNumericStorageId(), $paths, false);
108113
}
114+
115+
/**
116+
* Construct a search operator that filters out any paths that the current user doesn't have read permissions for
117+
*/
118+
private function getSearchFilter(): ISearchOperator {
119+
$forbiddenPaths = $this->aclManager->getForbiddenPaths($this->getNumericStorageId(), '');
120+
121+
$filters = array_map(fn (string $path) => new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [
122+
new SearchComparison(ISearchComparison::COMPARE_LIKE_CASE_SENSITIVE, 'path', SearchComparison::escapeLikeParameter($path) . '/%')
123+
]), $forbiddenPaths);
124+
$filters[] = new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_NOT, [
125+
new SearchComparison(ISearchComparison::COMPARE_IN, 'path', $forbiddenPaths)
126+
]);
127+
return new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, $filters);
128+
}
129+
130+
#[\Override]
131+
public function getQueryFilterForStorage(): ISearchOperator {
132+
$storageFilter = parent::getQueryFilterForStorage();
133+
134+
return new SearchBinaryOperator(
135+
ISearchBinaryOperator::OPERATOR_AND,
136+
[
137+
$storageFilter,
138+
$this->getSearchFilter(),
139+
]
140+
);
141+
}
109142
}

β€Žlib/ACL/ACLManager.phpβ€Ž

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,4 +293,23 @@ public function getBasePermission(int $folderId): int {
293293

294294
return Constants::PERMISSION_ALL;
295295
}
296+
297+
/**
298+
* Calculate all paths that the current user doesn't have access to.
299+
*
300+
* @return list<string>
301+
*/
302+
public function getForbiddenPaths(int $storageId, string $prefix): array {
303+
$rules = $this->ruleManager->getRulesForPrefix($this->user, $storageId, $prefix);
304+
$forbidden = [];
305+
306+
foreach ($rules as $path => $pathRules) {
307+
$mergedRule = Rule::mergeRules($pathRules);
308+
if ($mergedRule->applyPermissions(Constants::PERMISSION_READ) === 0) {
309+
$forbidden[] = $path;
310+
}
311+
}
312+
313+
return $forbidden;
314+
}
296315
}

β€Žlib/ACL/RuleManager.phpβ€Ž

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,16 +280,19 @@ public function getRulesForPrefix(IUser $user, int $storageId, string $prefix):
280280
$query->select(['f.fileid', 'mapping_type', 'mapping_id', 'mask', 'a.permissions', 'f.path'])
281281
->from('group_folders_acl', 'a')
282282
->innerJoin('a', 'filecache', 'f', $query->expr()->eq('f.fileid', 'a.fileid'))
283-
->where($query->expr()->orX(
284-
$query->expr()->like('f.path', $query->createNamedParameter($this->connection->escapeLikeParameter($prefix) . '/%')),
285-
$query->expr()->eq('f.path_hash', $query->createNamedParameter(md5($prefix)))
286-
))
287-
->andWhere($query->expr()->eq('f.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
283+
->where($query->expr()->eq('f.storage', $query->createNamedParameter($storageId, IQueryBuilder::PARAM_INT)))
288284
->andWhere($query->expr()->orX(...array_map(fn (IUserMapping $userMapping): ICompositeExpression => $query->expr()->andX(
289285
$query->expr()->eq('mapping_type', $query->createNamedParameter($userMapping->getType())),
290286
$query->expr()->eq('mapping_id', $query->createNamedParameter($userMapping->getId()))
291287
), $userMappings)));
292288

289+
if ($prefix !== '') {
290+
$query = $query->andWhere($query->expr()->orX(
291+
$query->expr()->like('f.path', $query->createNamedParameter($this->connection->escapeLikeParameter($prefix) . '/%')),
292+
$query->expr()->eq('f.path_hash', $query->createNamedParameter(md5($prefix)))
293+
));
294+
}
295+
293296
$rows = $query->executeQuery()->fetchAll();
294297

295298
/** @var list<array{fileid: int|string, mapping_type: 'circle'|'group'|'user', mapping_id: string, mask: int|string, permissions: int|string, path: string}> $rows */

0 commit comments

Comments
Β (0)