Skip to content

Reflect public shares in isPublic flag and fix permission check - #55147

Merged
salmart-dev merged 4 commits into
masterfrom
fixPublicShares
Dec 3, 2025
Merged

Reflect public shares in isPublic flag and fix permission check#55147
salmart-dev merged 4 commits into
masterfrom
fixPublicShares

Conversation

@salmart-dev

@salmart-devsalmart-dev commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

Summary

Fixes some issues introduced in the PR above, namely:

  1. The boolean flag isPublicShare was false in an instance where it should have been set to true. Flipping it to reflect the publicity of the share would break a compatibility functionality (read more below)
  2. Creating a collection using nicknames in a public share failed due to using instanceof on a storage wrapped by the files_accesscontrol app, instead of the function instanceOfStorage which checks if the instance itself or any wrapped storages (or wrappers) are instance of the passed class.

Compatibility Issue

For compatibility reasons the API call PROPFIND http://nextcloud.local/public.php/webdav should return the properties of the files in a public share. The share is identified through its ID, sent through the authorization header as username. In the master code, flipping the bool isPublicShare to true would break this functionality by changing the way the DAV tree is set up.

Checklist

@salmart-devsalmart-dev self-assigned this Sep 16, 2025
@salmart-devsalmart-dev added bug 3. to review Waiting for reviews labels Sep 16, 2025
@salmart-dev
salmart-devforce-pushed the fixPublicShares branch 2 times, most recently from 9535adf to ce8e8bdCompareOctober 16, 2025 09:55
@salmart-devsalmart-dev changed the title Fix public sharesReflect public shares in isPublic flag and fix permission checkOct 16, 2025
@salmart-dev
salmart-dev marked this pull request as ready for review October 16, 2025 13:43
@salmart-dev
salmart-dev requested a review from a team as a code ownerOctober 16, 2025 13:43
@salmart-dev
salmart-dev requested review from ArtificialOwl, CarlSchwan and leftybournes and removed request for a teamOctober 16, 2025 13:43
Comment threadapps/dav/lib/Connector/Sabre/ServerFactory.php Outdated

@provokateurinprovokateurin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but can you rebase and test everything again, as I fixed some things with chunked upload recently? I just want to avoid a regression with these changes.

Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
The isPublicShare was set to false in one instance where it should have
been true. Flipping the value to true, would break the functionality for
PROPFIND /public.php/webdav/ which returns properties of files in a
share identified by the username being the share token.
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
instanceof cannot be used to check the instance of a storage, doing so
breaks the check in certain cases. In this case, enabling the
`files_accesscontrol` app breaks the check.
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
Signed-off-by: Salvatore Martire <4652631+salmart-dev@users.noreply.github.com>
@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

LGTM, but can you rebase and test everything again, as I fixed some things with chunked upload recently? I just want to avoid a regression with these changes.

Tested:

  • can't PROPFIND on a public share without read permissions (files drop)
  • can PROPFIND on a share via /public.php/webdav with username = share ID (while on master I can't when setting isPublic to true)
  • can upload a collection with the files_accesscontrol app enabled, while on master I get the error "Target folder does not exist anymore"

So it seems all is still good 👍

@salmart-devsalmart-dev added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 3, 2025
@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport to stable32

@salmart-dev

Copy link
Copy Markdown
ContributorAuthor

/backport to stable31

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

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@salmart-dev@CarlSchwan@provokateurin