Skip to content

fix: use owner instead of current user for move-to-trash logic #3543

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.

Sign up for GitHub

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

Jump to bottom
Merged
AndyScherzinger merged 1 commit into from
Feb 6, 2025
Merged

fix: use owner instead of current user for move-to-trash logic #3543

AndyScherzinger merged 1 commit into from
Feb 6, 2025

Conversation

@icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Jan 17, 2025

fixes #3496

@icewind1991 icewind1991 added the 3. to review Items that need to be reviewed label Jan 17, 2025
@icewind1991 icewind1991 added this to the Nextcloud 31 milestone Jan 17, 2025
@icewind1991 icewind1991 requested review from a team, nfebe, provokateurin and yemkareems and removed request for a team January 17, 2025 15:02
@icewind1991
Copy link
Member Author

icewind1991 commented Jan 17, 2025

/backport to stable30

@icewind1991
Copy link
Member Author

icewind1991 commented Jan 17, 2025

/backport to stable29

@icewind1991 icewind1991 mentioned this pull request Jan 17, 2025
Closed
provokateurin
provokateurin approved these changes Jan 17, 2025
View reviewed changes
Copy link
Member

@provokateurin provokateurin left a comment

Choose a reason for hiding this comment

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

LGTM, but a test would be nice

@provokateurin provokateurin mentioned this pull request Jan 17, 2025
Closed
@kyteinsky
Copy link

kyteinsky commented Jan 17, 2025

maybe this fix could be extended to this line too for when anonymous users try to delete files. See #3544

@icewind1991
Copy link
Member Author

icewind1991 commented Jan 17, 2025

$user is supposed to be the account doing the deletion, not the owner

@kyteinsky
Copy link

kyteinsky commented Jan 17, 2025

yeah, the $user is used for the value of deleted_by and assumed to be a valid user id at some places so it doesn't seem to be trivial to fix. Better to address that separately.

@AndyScherzinger
Copy link
Member

AndyScherzinger commented Feb 6, 2025

/backport to stable31

@AndyScherzinger AndyScherzinger merged commit 4ac49eb into master Feb 6, 2025
44 checks passed
@AndyScherzinger AndyScherzinger deleted the trash-use-owner branch February 6, 2025 10:05
This was referenced Feb 6, 2025
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndyScherzinger AndyScherzinger AndyScherzinger approved these changes

@provokateurin provokateurin provokateurin approved these changes

@nfebe nfebe Awaiting requested review from nfebe nfebe was automatically assigned from nextcloud/server-backend

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems was automatically assigned from nextcloud/server-backend

Assignees

No one assigned

Labels

3. to review Items that need to be reviewed

Projects

None yet

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

Unable to delete a file/folder after switching to NC 30.0.4 and Group Folder 18.0.8 ("Delete file" action failed)

5 participants

@icewind1991 @kyteinsky @AndyScherzinger @provokateurin