[Bug]: Can rename file/folder in group folder without delete permission

luka-nextcloud commented 2 months ago

⚠️ This issue respects the following points: ⚠️

[X] This is a bug, not a question or a configuration/webserver/proxy issue.
[X] This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
[X] Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
[X] I agree to follow Nextcloud's Code of Conduct.

Bug description

Users can rename a file/folder when the right of delete file is denied in advanced authorization. Happens on 29 and master.

Steps to reproduce

Create a group folder "GroupFolder1"
Create a group "Group1"
Assign user "UserA" to group "Group1"
Assign group "Group1" to "GroupFolder1"
Enable "advanced permissions" for group folder "GroupFolder1" and select "Group1"
Login as "UserA"
Access files
Open sharing tab of folder "GroupFolder1"
Add advanced permission for group "Group1". Allow read, write, create, share. Deny delete
Access folder "GroupFolder1"
Create any file/folder
Rename created file/folder

Expected behavior

File/Folder cannot be renamed without delete permission.

Installation method

None

Nextcloud Server version

29

Operating system

None

PHP engine version

None

Web server

None

Database engine version

None

Is this bug present after an update or on a fresh install?

None

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

[ ] Default user-backend (database)
[ ] LDAP/ Active Directory
[ ] SSO - SAML
[ ] Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

small1 commented 2 months ago

This will be a tricky one since DELETE is not issued on rename. as long as you have write you can rename.

luka-nextcloud commented 2 months ago

This will be a tricky one since DELETE is not issued on rename. as long as you have write you can rename.

Some storages might have only one operation for rename, but that is not guaranteed. So, we should make it consistent between storages.

joshtrichards commented 1 month ago

Is this the inverse of nextcloud/groupfolders#859? :-)

Also see nextcloud/groupfolders#1646

provokateurin commented 1 month ago

Hm I'd say this is intended. Delete means non-recoverable changes while renaming a file or folder can be reverted easily. I'd say this can be fixed with https://github.com/nextcloud/groupfolders/issues/1646 by just adding it to the ACL options. I'm not sure if it should be the same as the "Move" ACL or a separate "Rename" ACL. @joshtrichards what do you think?

nextcloud / groupfolders