Skip to content

Conversation

@ArtificialOwl
Copy link
Member

@ArtificialOwl ArtificialOwl commented Nov 25, 2025

If coming from a federated folder, the share token is used to auth the webdav delete request.
Before creating new entry in the trashbin table, checking if file was deleted remotely using IRequest.
In case the token is a valid share token, using sharedWith to set deletedBy.
However deletedBy is now a remote entity but front-end will still displays 'Unknown', which is still better than 'You'

image

@ArtificialOwl ArtificialOwl force-pushed the fix/noid/real-account-on-deleted-federation branch 2 times, most recently from 225735b to 690c878 Compare November 25, 2025 17:11
@ArtificialOwl ArtificialOwl marked this pull request as ready for review November 26, 2025 09:18
@ArtificialOwl ArtificialOwl requested a review from a team as a code owner November 26, 2025 09:18
@ArtificialOwl ArtificialOwl requested review from Altahrim, salmart-dev and yemkareems and removed request for a team November 26, 2025 09:18
@ArtificialOwl ArtificialOwl force-pushed the fix/noid/real-account-on-deleted-federation branch from 690c878 to ce32c06 Compare November 26, 2025 09:19
@ArtificialOwl ArtificialOwl force-pushed the fix/noid/real-account-on-deleted-federation branch from ce32c06 to ea8b133 Compare December 1, 2025 17:25
try {
$request = Server::get(IRequest::class);
/** @psalm-suppress NoInterfaceProperties */
$token = $request->server['PHP_AUTH_USER'] ?? '';
Copy link
Member

Choose a reason for hiding this comment

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

Would this somehow allow forging the value, so it looks like someone else deleted the file?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the webdav request is authed, it might be by using the share token.
I only check if this is the case so I can assign a name to the deleter.

Yes, if you know the share token, you can delete the file (if the share has permission), but this is the expected behavior

@ArtificialOwl
Copy link
Member Author

/backport to stable32

@ArtificialOwl
Copy link
Member Author

/backport to stable31

This was referenced Dec 4, 2025
@ArtificialOwl ArtificialOwl merged commit b18372e into master Dec 4, 2025
231 of 247 checks passed
@ArtificialOwl ArtificialOwl deleted the fix/noid/real-account-on-deleted-federation branch December 4, 2025 14:46
@AndyScherzinger AndyScherzinger added this to the Nextcloud 33 milestone Dec 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants