BookStackApp / BookStack

A platform to create documentation/wiki content built with PHP & Laravel
https://www.bookstackapp.com/
MIT License
15.45k stars 1.94k forks source link

Custom Permission should not overwrite owner's permission #5329

Closed im-Kitsch closed 5 days ago

im-Kitsch commented 6 days ago

Describe the Bug

Hi, @ssddanbrown , first of all, thanks for your great work.

I wonder if you agree that the project(e.g. book/shelf, etc) owner's ownership should always be respected in bookstack.

But currently, as the following example shows, the owner's permission would always be overwritten by custom permission setting, i.e. the owner permission is botoom tier while it should be anyway first-class-citizen.

I don't think this is an expected behavior. The owner should not lost the access to its own book/shelf.

This related to many discussion like #2697 #2903 #3185 #3577 .

Steps to Reproduce

  1. in one bookstack instance, create a user 'test_user' and assign the user to Editor Role, set Editor Role has permission to mange the permission of its own books, shelves.
  2. login as test_user,
  3. create shelve test_shelve
  4. change the permission of test_shelve, set Editor goup has no permission for editing/viewing for this shelve
image

Expected Behaviour

the test_shelve become orphan and is not visible to its owner anymore.

Discussion

I think you regard it as a expected feature, but shouldn't it be regarded as a bug?

While implementing fine-grained permission control, the goal is to allow each book to have individual permissions and visibilities. But does it have complete converage?

For a simple example, assume by default the Viewer role could view all books, if one user under viewer group wants his book to be invisible to others, the specific group permission setting also diables the access of owner. If the owner wants to keep ownership, there is only one choice: by default all groups could only view/edit/update/delete own project and **add permission in specific permission setting if needed***. In this view the permission setting has incomplete converage and there is only one possible default setting.

To solve it, Currently in the permission model has three levels,

Shall it have a higher level owner that has highest priority? That would be more natrual.

Exact BookStack Version

24.05.4

ssddanbrown commented 5 days ago

Hi @im-Kitsch, I'm going to close this off as a duplicate of existing issue #2697 since I feel they share the same fundamental desire/wish.

Ultimately this comes down to expectations which may differ depending on use, context and environment. I personally wouldn't expect ownership permissions to override specifically set item-level permissions, which is therefore the logic I've implemented, but I respect that some may feel/desire differently.

Either way, I wouldn't want to just change this since it'd be a considerable breaking change to visibility so, if ever supported in any way, it'd have to be an option (or somehow compatible with the existing logic).