BookStack icon indicating copy to clipboard operation
BookStack copied to clipboard

Guest only accepts custom permissions from role Public

Open Bolthier opened this issue 7 years ago • 1 comments

Describe the bug Guest user only accepts custom permissions from the role Public. Any other role permission is ignored. The default permissions however work if given from another role than Public.

Steps To Reproduce

  1. Go to roles and create another role "BookA", add this role to the user Guest
  2. Create a book "BookA" and configure custom permissions for role BookA to view and none for role Public
  3. Open a private tab with the book
  4. See no book

Expected behavior Guest user should inherit custom permissions from other roles than Public.

Your Configuration (please complete the following information):

  • Exact BookStack Version: BookStack v0.25.0
  • PHP Version: 7.2.10
  • Hosting Method: Apache2

Additional context I use custom permissions for all of my books. For every book I use a different role with the same name as the book. Only this role can create or edit. If I want users to edit a book, I just add them to the role and I'm done with it. This way I can also easily overview any given edit permissions of a user.

This works pretty flawless except for the Guest user, which I want to give edit access to some books but without loosing track where the role Public has permission to edit. This can get icky with an expanding library.

Bolthier avatar Jan 19 '19 17:01 Bolthier

Sorry for the very late response, Just looking through old issues and came across this and decided to check.

Can confirm this is the case, Is due to this condition: https://github.com/BookStackApp/BookStack/blob/9490457d044b51fbe330998ce37dcfe255038f55/app/Auth/Permissions/PermissionService.php#L114

Might be something we should address during permission system review so "breaking" changes will be wrapped up in one go. Will be dangerous to just go ahead and update the logic, since this has likely been in place since the early days of the permission system. I'd count that as a breaking change as may expose content not previously thought as exposed.

To do this safely I'd suggest we, on change of this logic:

  • Drop existing secondary (Non-public) roles from the guest user via database migration.
    • This will remove potentially used system permissions and hence be a breaking change still.
      • These could be easily re-applied under the caution of the considerations after upgrade.
    • This would safely disconnect any entity-permissions which may become active after logic change.

ssddanbrown avatar May 17 '22 12:05 ssddanbrown

Apologies in the extra year of delay for this, I has missed this during permission system changes. I've now applied the relevant changes in 777027bc48d628f1283b0a734ffabe5dbabebc36. This will be part of the next feature release.

As I planned above, additional assigned guest user roles will be detached upon upgrade for safety. These could have still applied some active permissions, so this will be a potential breaking change of access state, but should be easy for admins to re-assign after update where needed. Will have a breaking change advisory to advise of this for the feature release.

Thanks @Bolthier for raising.

Docs Updates

  • [x] - Upgrade notice to advise of changes and drop of additional guest user roles.

ssddanbrown avatar Jun 10 '23 10:06 ssddanbrown