Skip to content

filter ACL forbidden paths from search query#4745

Merged
icewind1991 merged 3 commits into
masterfrom
acl-search-filtering
Jun 11, 2026
Merged

filter ACL forbidden paths from search query#4745
icewind1991 merged 3 commits into
masterfrom
acl-search-filtering

Conversation

@icewind1991

Copy link
Copy Markdown
Member

Currently, the logic for removing files the user doesn't have access to from search is done as a post-processing step.

While this filters out the correct entries, it means that the number of returned results is less than the limit asked for (or even 0). Which the search logic interprets as there being no more results.

By applying the filtering as part of the query, we return the number of results asked for.

@icewind1991 icewind1991 added this to the Nextcloud 34 milestone Jun 4, 2026
@icewind1991 icewind1991 requested a review from provokateurin June 4, 2026 19:25
@icewind1991 icewind1991 added the 3. to review Items that need to be reviewed label Jun 4, 2026
@icewind1991

Copy link
Copy Markdown
Member Author

Fixes nextcloud/server#60306

Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991

Copy link
Copy Markdown
Member Author

/backport to stable34

@icewind1991

Copy link
Copy Markdown
Member Author

/backport to stable33

@icewind1991 icewind1991 merged commit d9dfef8 into master Jun 11, 2026
56 checks passed
@icewind1991 icewind1991 deleted the acl-search-filtering branch June 11, 2026 14:02
@danxuliu

Copy link
Copy Markdown
Member

@icewind1991 @provokateurin While the fix solves the issue for users with forbidden paths it breaks searching for users without any forbidden path. It can be reproduced with the steps from nextcloud/server#60306 but searching as the user manager instead of as the user restricted. The E2E test from #4675 also fails for the search done as user manager.

Adding something like:

		if (empty($forbiddenPaths)) {
			return new SearchBinaryOperator(ISearchBinaryOperator::OPERATOR_AND, []);
		}

to getSearchFilter() make it work as expected for both manager and restricted, but I did not dig into the issue.

Besides that, would it be possible to backport this pull request and the follow up also to stable32? Thanks!

@icewind1991

Copy link
Copy Markdown
Member Author

/backport to stable32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Items that need to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants