Skip to content
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.

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

fix: Skip registering mount points for missing owner node #37736

Closed
wants to merge 1 commit into from

Conversation

juliushaertl
Copy link
Member

@juliushaertl juliushaertl commented Apr 14, 2023

Make sure that we don't show share mount points where the owner node is not available anymore.

steps to reproduce:

  • share a file with user1 (share_with) as user2 (uid_initiator) owned by user3 (uid_owner)
  • create group1
  • add user2 and user3 as members to group1
  • create groupfolder for group1
  • as user3, move the file into the new groupfolder
  • the share still has user1 as share_with, user2 as uid_initiator and user3 as uid_owner and is accessible
  • remove user3 from group1

🛑 the shared file is still shown but user1 cannot access the file anymore

This still leaves an orphaned share entry but so far I had no luck finding a proper sql query so that we could add this to

class DeleteOrphanedSharesJob extends TimedJob {
however skipping the share mount will at least avoid broken files being shown to users.

@szaimen szaimen added the 2. developing Work in progress label Apr 14, 2023
@szaimen szaimen added this to the Nextcloud 27 milestone Apr 14, 2023
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

The root problem is in the move logic no, it should somehow update the share info?

Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

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

This is a pretty expensive operation to perform for each incoming share and will kill performance for users with a large number of incoming shares.

@icewind1991
Copy link
Member

It's probably better to catch this error when the user tries to interact with the broken. That way it doesn't have to run this check for N shares

This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
@blizzz blizzz mentioned this pull request Nov 14, 2023
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Mar 28, 2024
@skjnldsv skjnldsv added this to the Nextcloud 30 milestone Mar 28, 2024
@juliushaertl
Copy link
Member Author

I lack time to finish this, so I'll close for now. We can reopen anyways if we need.

@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@skjnldsv skjnldsv deleted the bugfix/noid/unavailable-shares branch August 30, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants