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(ACL): don't put inherited ACL permissions in the propPatch request payload #2660

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Dec 8, 2023

⚠️ As #2637 is not solved yet, tested it on stable26 and stable27 only

Before After
create ACL image
image image
remove ACL image
image image

…t payload

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy added bug 3. to review Items that need to be reviewed javascript Items related to Javascript code feature: acl Items related to the groupfolders ACL or "Advanced Permissions" labels Dec 8, 2023
@Antreesy Antreesy added this to the Nextcloud 28 milestone Dec 8, 2023
@Antreesy Antreesy self-assigned this Dec 8, 2023
@Antreesy
Copy link
Contributor Author

Antreesy commented Dec 8, 2023

/backport to stable27

@Antreesy
Copy link
Contributor Author

Antreesy commented Dec 8, 2023

/backport to stable26

@Antreesy
Copy link
Contributor Author

Antreesy commented Dec 8, 2023

/backport to stable28

@come-nc
Copy link
Contributor

come-nc commented Dec 12, 2023

@Antreesy Hard to test this before UI is fixed. Ideally I would like a cypress test for this now that we have cypress setup, but it will need the UI to work as well I suppose.

@Antreesy
Copy link
Contributor Author

it will need the UI to work as well I suppose.

Indeed, that would require sidebar view to test it. Now, with cypress we do everything with occ commands, one rule at the time, so impoossible to reproduce this error.

ATM web interface sends all rules together, which are handled by ACLPlugin propPatch method:

public function propPatch(string $path, PropPatch $propPatch): void {

As seen from there, it doesn't divide coming rules to inherited and local, consider them all as local, so we shouldn't pass inherited unless they haven't been changed.

Copy link

@GretaD GretaD left a comment

Choose a reason for hiding this comment

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

code looks good but i didnt test it

@mejo-

This comment was marked as outdated.

Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Please disregard my last comment. For some reason I didn't run the code with your commit. Now that I fixed this, it works as expected 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Items that need to be reviewed bug feature: acl Items related to the groupfolders ACL or "Advanced Permissions" javascript Items related to Javascript code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inherited ACL permissions for groups in subfolder converted to local after adding new ACL rules
4 participants