BookStackApp / BookStack

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

Add the option for new books to inherit permissions when created #5205

Open anbcodes opened 2 months ago

anbcodes commented 2 months ago

Adds an option to bookshelves to copy permissions to books created under them. This fixes the issue where someone has permission to create a book but cannot view the book they created. This fixes #1596 by implementing the suggestion in this comment.

ssddanbrown commented 2 months ago

Thanks for offering this @anbcodes but I'm really not keen on introducing layers of settings for process control like this. From an implementation point of view I'd be more open to a single option during book creation, but even that adds complication when considered with expectations around permission control, and I'm not sure it meets the desired process of many.

Part me of things we should just document a logical theme solution on our hacks since that could provide a flexible base to be used & customized without complicating anything in the core platform.

intellasoft commented 2 months ago

Hi @ssddanbrown,

I'm helping coordinate this change that @anbcodes made that we badly need. Right now it's impossible for our team to effectively use bookstack without this change. And I'm not the only one with this issue, given the popularity of https://github.com/BookStackApp/BookStack/issues/1596

Default Behaviors when you segment different permissions to various shelves

As much as I am completely in love with Bookstack... we had none of these issues with our old system.

There's already a button to do this process manually. Why not make it automatic for those that need it? I know we're certainly not alone in needing automatic permissions, especially when there's a 1:1:1 shelf,book,page and nothing is re-used anywhere sort of situation most people find themselves in.

ssddanbrown commented 2 months ago

Why not make it automatic for those that need it?

@intellasoft I'm not against the idea, just not keen on adding layers of process specific setting, especially as they would still not solve all desires or assumptions in the behaviour here, and could even introduce new considerations/complications.

Is there any reason using a logical theme hack would not be suitable in your scenario given that you badly require this functionality?

intellasoft commented 2 months ago

It sounds like if there were some hooks for create book to then run a plugin for whatever the user wants to do.. that would simplify the core.

The hacks method sounds.... well... hackey. Ie: having to drop in code or replace a script for each version does not sound ideal.

I'm a huge fan of callbacks and plugin hooks. Maybe we should rework this to use new hooks...

ssddanbrown commented 2 months ago

@intellasoft the logical theme system (which would be used for such a hack) is essentially a hook system. The code you use within the hooks is not deemed supported/stable, but needing to update would not be common.

You don't have to drop-in code or replace scripts on each update, the code is placed in its own folder, and does not have to be re-applied on each BookStack change.

We emit a bookshelf-update then books are added via the shelf > create-book flow, so it should be possible to hook into that event, find newly assigned & created books, then copy permissions to them. This comment reflects a very similar solution (although depending on slightly less stable session/process-based internal functionality).

intellasoft commented 2 months ago

Ah got it. And then... is there a hook to add additional configuration elements to the portal?

intellasoft commented 2 months ago

But... I also disagree (as a user) that this should be a plugin.

I don't get the design decision that would allow for utilizing the permission system in such a way that the built-in inherent behaviors make the system unusable for affected users. Especially since the (manual) process already exists. We're just automating it.

Like.. what's the purpose of allowing a book to be created that the user then has no access to?

ssddanbrown commented 2 months ago

And then... is there a hook to add additional configuration elements to the portal?

No. We don't generally provide hooks for UI components but, depending on what you might need to do, you may be able to override UI using the visual theme system.

Like.. what's the purpose of allowing a book to be created that the user then has no access to?

The permission system is complex and there are likely naturally many scenarios where you could encounter logic that does not suit your process, especially where the system is used with different intentions from their original design (using shelves as top-level permission containers / owners of books, using permissions to open access rather than to lock access). It's not really a goal to ensure every one of these scenarios is addressed via options, since that's a road to added complexity and often would introduce complications of their own.

I'd rather provide flexible customization solutions (logical theme system) for those that need it rather than complicate the core system to have options for the different processes, locking shelves further into a role they're not really built/intended for.