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.

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

fix: consider all paths the user has for a share source when considering max permissions #44791

Merged
merged 2 commits into from
May 13, 2024
Merged

fix: consider all paths the user has for a share source when considering max permissions #44791

merged 2 commits into from
May 13, 2024

Conversation

icewind1991
Copy link
Member

@icewind1991 icewind1991 commented Apr 11, 2024 •
edited
Loading

To test

  • Create an external storage "Storage" for all users with sharing enabled
  • As user1 create folder "Storage/folder" and share it read-only with user2
  • As user2 attempt to share "Storage/folder" with edit permissions to user3

@icewind1991 icewind1991 added the 2. developing Work in progress label Apr 11, 2024
@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
@icewind1991 icewind1991 force-pushed the reshare-permission-logic branch from 326c31f to 6980dae Compare May 2, 2024 14:07
@icewind1991 icewind1991 mentioned this pull request May 2, 2024
Closed
@icewind1991 icewind1991 force-pushed the reshare-permission-logic branch 6 times, most recently from 5756ffd to 8ce01e1 Compare May 3, 2024 12:44
@icewind1991 icewind1991 changed the title fix: use the correct source share for getting the max reshare permissions fix: consider all paths the user has for a share source when considering max permissions May 3, 2024
@icewind1991
fix: cleanup logic for getting the max reshare permissions
4c085ef 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the reshare-permission-logic branch from 8ce01e1 to 4c085ef Compare May 3, 2024 12:53
@icewind1991 icewind1991 marked this pull request as ready for review May 3, 2024 13:30
@icewind1991
test: adjust test to new permissions check logic
4a72b96 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the reshare-permission-logic branch from 9b0a812 to 4a72b96 Compare May 3, 2024 13:39
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, Altahrim and yemkareems and removed request for a team May 3, 2024 14:22
@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels May 3, 2024
come-nc
come-nc approved these changes May 7, 2024
View reviewed changes
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.

I cannot find it but you made a PR not long ago to use the node the user tried to share instead of the first one when checking reshare permission, is that a different fix for the same thing or two different cases?

@come-nc
Copy link
Contributor

come-nc commented May 7, 2024

I cannot find it but you made a PR not long ago to use the node the user tried to share instead of the first one when checking reshare permission, is that a different fix for the same thing or two different cases?

Ok I’m not crazy I think you forced push over it in this very PR 🙈

@icewind1991
Copy link
Member Author

icewind1991 commented May 13, 2024 •
edited
Loading

I cannot find it but you made a PR not long ago to use the node the user tried to share instead of the first one when checking reshare permission, is that a different fix for the same thing or two different cases?

Ok I’m not crazy I think you forced push over it in this very PR 🙈

Yeah that approach didn't end up working with in some cases

@icewind1991 icewind1991 merged commit d76de94 into master May 13, 2024
157 checks passed
@icewind1991 icewind1991 deleted the reshare-permission-logic branch May 13, 2024 14:00
@icewind1991
Copy link
Member Author

/backport to stable29

@icewind1991
Copy link
Member Author

/backport to stable28

@backportbot backportbot bot mentioned this pull request May 13, 2024
Merged
@backportbot backportbot bot mentioned this pull request May 13, 2024
Merged
2 tasks
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Altahrim Altahrim Altahrim approved these changes

@come-nc come-nc come-nc approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl 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 Waiting for reviews
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.
4 participants
@icewind1991 @come-nc @Altahrim @susnux