From f19bad89033ee31e9157341214ae3a7e3b0fbb40 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 2 Oct 2022 13:17:28 +0100
Subject: [PATCH 01/19] Started item permission design revamp
---
resources/icons/role.svg | 4 +++
resources/sass/_components.scss | 31 ++++++++++++++++---
resources/sass/_layout.scss | 4 +--
resources/sass/_spacing.scss | 14 ++++++++-
.../form/entity-permissions-row.blade.php | 28 +++++++++++++++++
.../views/form/entity-permissions.blade.php | 25 ++-------------
6 files changed, 77 insertions(+), 29 deletions(-)
create mode 100644 resources/icons/role.svg
create mode 100644 resources/views/form/entity-permissions-row.blade.php
diff --git a/resources/icons/role.svg b/resources/icons/role.svg
new file mode 100644
index 000000000..e7cad506d
--- /dev/null
+++ b/resources/icons/role.svg
@@ -0,0 +1,4 @@
+<svg viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg">
+ <path d="M0 0h24v24H0z" fill="none"/>
+ <path d="M16 11c1.66 0 2.99-1.34 2.99-3S17.66 5 16 5c-1.66 0-3 1.34-3 3s1.34 3 3 3zm-8 0c1.66 0 2.99-1.34 2.99-3S9.66 5 8 5C6.34 5 5 6.34 5 8s1.34 3 3 3zm0 2c-2.33 0-7 1.17-7 3.5V19h14v-2.5c0-2.33-4.67-3.5-7-3.5zm8 0c-.29 0-.62.02-.97.05 1.16.84 1.97 1.97 1.97 3.45V19h6v-2.5c0-2.33-4.67-3.5-7-3.5z"/>
+</svg>
\ No newline at end of file
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index c00f57954..9f737f3be 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -798,11 +798,34 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
max-width: 500px;
}
-.permissions-table [permissions-table-toggle-all-in-row] {
- display: none;
+.content-permissions {
+ box-shadow: 0 1px 3px 0 rgba(0, 0, 0, 0.1);
}
-.permissions-table tr:hover [permissions-table-toggle-all-in-row] {
- display: inline;
+.content-permissions-row {
+ border: 1.5px solid #E2E2E2;
+ border-bottom-width: 0;
+ label {
+ padding-bottom: 0;
+ }
+ &:hover {
+ background-color: #F2F2F2;
+ }
+}
+.content-permissions-row:first-child {
+ border-radius: 4px 4px 0 0;
+}
+.content-permissions-row:last-child {
+ border-radius: 0 0 4px 4px;
+ border-bottom-width: 1.5px;
+}
+.content-permissions-row-toggle-all {
+ visibility: hidden;
+}
+.content-permissions-row:hover .content-permissions-row-toggle-all {
+ visibility: visible;
+}
+.content-permissions-row-label {
+ font-weight: bold;
}
.template-item {
diff --git a/resources/sass/_layout.scss b/resources/sass/_layout.scss
index 2cd57d496..cfb8397c9 100644
--- a/resources/sass/_layout.scss
+++ b/resources/sass/_layout.scss
@@ -158,8 +158,8 @@ body.flexbox {
}
}
-.gap-m {
- gap: $-m;
+.flex-none {
+ flex: none;
}
.justify-flex-start {
diff --git a/resources/sass/_spacing.scss b/resources/sass/_spacing.scss
index 40217de9b..14f8918dc 100644
--- a/resources/sass/_spacing.scss
+++ b/resources/sass/_spacing.scss
@@ -29,4 +29,16 @@
}
}
@include spacing('margin', 'm');
-@include spacing('padding', 'p');
\ No newline at end of file
+@include spacing('padding', 'p');
+
+@each $sizeLetter, $size in $spacing {
+ .gap-#{$sizeLetter} {
+ gap: $size !important;
+ }
+ .gap-x-#{$sizeLetter} {
+ column-gap: $size !important;
+ }
+ .gap-y-#{$sizeLetter} {
+ row-gap: $size !important;
+ }
+}
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
new file mode 100644
index 000000000..023aa36d2
--- /dev/null
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -0,0 +1,28 @@
+<div class="content-permissions-row flex-container-row justify-space-between wrap">
+ <div class="content-permissions-row-label gap-x-m flex-container-row items-center px-l py-m flex">
+ <div class="text-large" title="{{ trans('common.role') }}">
+ @icon('role')
+ </div>
+ <span>{{ $role->display_name }}</span>
+ <button type="button"
+ class="ml-auto flex-none text-small text-primary text-button hover-underline content-permissions-row-toggle-all hide-under-s"
+ permissions-table-toggle-all-in-row
+ >{{ trans('common.toggle_all') }}</button>
+ </div>
+ <div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
+ <div class="px-l">
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view'])
+ </div>
+ <div class="px-l">
+ @if(!$model->isA('page'))
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create'])
+ @endif
+ </div>
+ <div class="px-l">
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update'])
+ </div>
+ <div class="px-l">
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete'])
+ </div>
+ </div>
+</div>
\ No newline at end of file
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 206955fe9..18df0bb69 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -24,31 +24,12 @@
<p class="text-warn">{{ trans('entities.shelves_permissions_cascade_warning') }}</p>
@endif
- <hr>
- <table permissions-table class="table permissions-table toggle-switch-list" style="{{ !$model->restricted ? 'display: none' : '' }}">
- <tr>
- <th>{{ trans('common.role') }}</th>
- <th colspan="{{ $model->isA('page') ? '3' : '4' }}">
- {{ trans('common.actions') }}
- <a href="#" permissions-table-toggle-all class="text-small ml-m text-primary">{{ trans('common.toggle_all') }}</a>
- </th>
- </tr>
+ <div class="content-permissions mt-m mb-xl">
@foreach(\BookStack\Auth\Role::restrictable() as $role)
- <tr>
- <td width="33%" class="pt-m">
- {{ $role->display_name }}
- <a href="#" permissions-table-toggle-all-in-row class="text-small float right ml-m text-primary">{{ trans('common.toggle_all') }}</a>
- </td>
- <td>@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view'])</td>
- @if(!$model->isA('page'))
- <td>@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create'])</td>
- @endif
- <td>@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update'])</td>
- <td>@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete'])</td>
- </tr>
+ @include('form.entity-permissions-row', ['role' => $role, 'model' => $model])
@endforeach
- </table>
+ </div>
<div class="text-right">
<a href="{{ $model->getUrl() }}" class="button outline">{{ trans('common.cancel') }}</a>
From b8b0afa0df6f9a63438c214fc08664e6b4cd3455 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 2 Oct 2022 13:57:32 +0100
Subject: [PATCH 02/19] Cleaned up old permission JS code
Removed now unused JS entity-permissions compontent.
Updated existing permissions-table compontent to newer format.
Removed now unused translation string.
---
.../components/entity-permissions-editor.js | 20 -------------
resources/js/components/index.js | 2 --
resources/js/components/permissions-table.js | 15 +++++-----
resources/lang/en/entities.php | 1 -
.../form/entity-permissions-row.blade.php | 6 ++--
.../views/form/entity-permissions.blade.php | 9 +-----
.../views/settings/roles/parts/form.blade.php | 30 +++++++++----------
7 files changed, 26 insertions(+), 57 deletions(-)
delete mode 100644 resources/js/components/entity-permissions-editor.js
diff --git a/resources/js/components/entity-permissions-editor.js b/resources/js/components/entity-permissions-editor.js
deleted file mode 100644
index a821792a0..000000000
--- a/resources/js/components/entity-permissions-editor.js
+++ /dev/null
@@ -1,20 +0,0 @@
-
-class EntityPermissionsEditor {
-
- constructor(elem) {
- this.permissionsTable = elem.querySelector('[permissions-table]');
-
- // Handle toggle all event
- this.restrictedCheckbox = elem.querySelector('[name=restricted]');
- this.restrictedCheckbox.addEventListener('change', this.updateTableVisibility.bind(this));
- }
-
- updateTableVisibility() {
- this.permissionsTable.style.display =
- this.restrictedCheckbox.checked
- ? null
- : 'none';
- }
-}
-
-export default EntityPermissionsEditor;
\ No newline at end of file
diff --git a/resources/js/components/index.js b/resources/js/components/index.js
index f360e2b0c..4b17dc403 100644
--- a/resources/js/components/index.js
+++ b/resources/js/components/index.js
@@ -18,7 +18,6 @@ import dropdown from "./dropdown.js"
import dropdownSearch from "./dropdown-search.js"
import dropzone from "./dropzone.js"
import editorToolbox from "./editor-toolbox.js"
-import entityPermissionsEditor from "./entity-permissions-editor.js"
import entitySearch from "./entity-search.js"
import entitySelector from "./entity-selector.js"
import entitySelectorPopup from "./entity-selector-popup.js"
@@ -75,7 +74,6 @@ const componentMapping = {
"dropdown-search": dropdownSearch,
"dropzone": dropzone,
"editor-toolbox": editorToolbox,
- "entity-permissions-editor": entityPermissionsEditor,
"entity-search": entitySearch,
"entity-selector": entitySelector,
"entity-selector-popup": entitySelectorPopup,
diff --git a/resources/js/components/permissions-table.js b/resources/js/components/permissions-table.js
index baad75258..df3c055ca 100644
--- a/resources/js/components/permissions-table.js
+++ b/resources/js/components/permissions-table.js
@@ -1,22 +1,21 @@
class PermissionsTable {
- constructor(elem) {
- this.container = elem;
+ setup() {
+ this.container = this.$el;
// Handle toggle all event
- const toggleAll = elem.querySelector('[permissions-table-toggle-all]');
- toggleAll.addEventListener('click', this.toggleAllClick.bind(this));
+ for (const toggleAllElem of (this.$manyRefs.toggleAll || [])) {
+ toggleAllElem.addEventListener('click', this.toggleAllClick.bind(this));
+ }
// Handle toggle row event
- const toggleRowElems = elem.querySelectorAll('[permissions-table-toggle-all-in-row]');
- for (let toggleRowElem of toggleRowElems) {
+ for (const toggleRowElem of (this.$manyRefs.toggleRow || [])) {
toggleRowElem.addEventListener('click', this.toggleRowClick.bind(this));
}
// Handle toggle column event
- const toggleColumnElems = elem.querySelectorAll('[permissions-table-toggle-all-in-column]');
- for (let toggleColElem of toggleColumnElems) {
+ for (const toggleColElem of (this.$manyRefs.toggleColumn || [])) {
toggleColElem.addEventListener('click', this.toggleColumnClick.bind(this));
}
}
diff --git a/resources/lang/en/entities.php b/resources/lang/en/entities.php
index 1720801d2..28ec591d7 100644
--- a/resources/lang/en/entities.php
+++ b/resources/lang/en/entities.php
@@ -43,7 +43,6 @@ return [
// Permissions and restrictions
'permissions' => 'Permissions',
'permissions_intro' => 'Once enabled, These permissions will take priority over any set role permissions.',
- 'permissions_enable' => 'Enable Custom Permissions',
'permissions_save' => 'Save Permissions',
'permissions_owner' => 'Owner',
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index 023aa36d2..bff7315a0 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -1,4 +1,4 @@
-<div class="content-permissions-row flex-container-row justify-space-between wrap">
+<div component="permissions-table" class="content-permissions-row flex-container-row justify-space-between wrap">
<div class="content-permissions-row-label gap-x-m flex-container-row items-center px-l py-m flex">
<div class="text-large" title="{{ trans('common.role') }}">
@icon('role')
@@ -6,7 +6,7 @@
<span>{{ $role->display_name }}</span>
<button type="button"
class="ml-auto flex-none text-small text-primary text-button hover-underline content-permissions-row-toggle-all hide-under-s"
- permissions-table-toggle-all-in-row
+ refs="permissions-table@toggle-all"
>{{ trans('common.toggle_all') }}</button>
</div>
<div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
@@ -14,7 +14,7 @@
@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view'])
</div>
<div class="px-l">
- @if(!$model->isA('page'))
+ @if(!$model instanceof \BookStack\Entities\Models\Page)
@include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create'])
@endif
</div>
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 18df0bb69..321e2f06c 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -1,16 +1,10 @@
-<form action="{{ $model->getUrl('/permissions') }}" method="POST" entity-permissions-editor>
+<form action="{{ $model->getUrl('/permissions') }}" method="POST">
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
<div class="grid half left-focus v-center">
<div>
<p class="mb-none mt-m">{{ trans('entities.permissions_intro') }}</p>
- <div>
- @include('form.checkbox', [
- 'name' => 'restricted',
- 'label' => trans('entities.permissions_enable'),
- ])
- </div>
</div>
<div>
<div class="form-group">
@@ -24,7 +18,6 @@
<p class="text-warn">{{ trans('entities.shelves_permissions_cascade_warning') }}</p>
@endif
-
<div class="content-permissions mt-m mb-xl">
@foreach(\BookStack\Auth\Role::restrictable() as $role)
@include('form.entity-permissions-row', ['role' => $role, 'model' => $model])
diff --git a/resources/views/settings/roles/parts/form.blade.php b/resources/views/settings/roles/parts/form.blade.php
index 593791997..044b4ceb4 100644
--- a/resources/views/settings/roles/parts/form.blade.php
+++ b/resources/views/settings/roles/parts/form.blade.php
@@ -26,9 +26,9 @@
</div>
</div>
- <div permissions-table>
+ <div component="permissions-table">
<label class="setting-list-label">{{ trans('settings.role_system') }}</label>
- <a href="#" permissions-table-toggle-all class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-all" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
<div class="toggle-switch-list grid half mt-m">
<div>
@@ -56,20 +56,20 @@
<p class="text-warn">{{ trans('settings.role_asset_admins') }}</p>
@endif
- <table permissions-table class="table toggle-switch-list compact permissions-table">
+ <table component="permissions-table" class="table toggle-switch-list compact permissions-table">
<tr>
<th width="20%">
- <a href="#" permissions-table-toggle-all class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-all" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</th>
- <th width="20%" permissions-table-toggle-all-in-column>{{ trans('common.create') }}</th>
- <th width="20%" permissions-table-toggle-all-in-column>{{ trans('common.view') }}</th>
- <th width="20%" permissions-table-toggle-all-in-column>{{ trans('common.edit') }}</th>
- <th width="20%" permissions-table-toggle-all-in-column>{{ trans('common.delete') }}</th>
+ <th width="20%" refs="permissions-table@toggle-column">{{ trans('common.create') }}</th>
+ <th width="20%" refs="permissions-table@toggle-column">{{ trans('common.view') }}</th>
+ <th width="20%" refs="permissions-table@toggle-column">{{ trans('common.edit') }}</th>
+ <th width="20%" refs="permissions-table@toggle-column">{{ trans('common.delete') }}</th>
</tr>
<tr>
<td>
<div>{{ trans('entities.shelves') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>
@include('settings.roles.parts.checkbox', ['permission' => 'bookshelf-create-all', 'label' => trans('settings.role_all')])
@@ -93,7 +93,7 @@
<tr>
<td>
<div>{{ trans('entities.books') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>
@include('settings.roles.parts.checkbox', ['permission' => 'book-create-all', 'label' => trans('settings.role_all')])
@@ -117,7 +117,7 @@
<tr>
<td>
<div>{{ trans('entities.chapters') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>
@include('settings.roles.parts.checkbox', ['permission' => 'chapter-create-own', 'label' => trans('settings.role_own')])
@@ -143,7 +143,7 @@
<tr>
<td>
<div>{{ trans('entities.pages') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>
@include('settings.roles.parts.checkbox', ['permission' => 'page-create-own', 'label' => trans('settings.role_own')])
@@ -169,7 +169,7 @@
<tr>
<td>
<div>{{ trans('entities.images') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>@include('settings.roles.parts.checkbox', ['permission' => 'image-create-all', 'label' => ''])</td>
<td style="line-height:1.2;"><small class="faded">{{ trans('settings.role_controlled_by_asset') }}<sup>1</sup></small></td>
@@ -187,7 +187,7 @@
<tr>
<td>
<div>{{ trans('entities.attachments') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>@include('settings.roles.parts.checkbox', ['permission' => 'attachment-create-all', 'label' => ''])</td>
<td style="line-height:1.2;"><small class="faded">{{ trans('settings.role_controlled_by_asset') }}</small></td>
@@ -205,7 +205,7 @@
<tr>
<td>
<div>{{ trans('entities.comments') }}</div>
- <a href="#" permissions-table-toggle-all-in-row class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
+ <a href="#" refs="permissions-table@toggle-row" class="text-small text-primary">{{ trans('common.toggle_all') }}</a>
</td>
<td>@include('settings.roles.parts.checkbox', ['permission' => 'comment-create-all', 'label' => ''])</td>
<td style="line-height:1.2;"><small class="faded">{{ trans('settings.role_controlled_by_asset') }}</small></td>
From a03245e427d3257eeb2bbf137e8e6ce1388c1e69 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 2 Oct 2022 18:09:48 +0100
Subject: [PATCH 03/19] Added user-interface for "Everyone Else" entity
permission item
Nothing on back-end logic done to hook this new option up.
Addition of permissions for role_id=0 works out of the box, but active
"everyone else" permissions, with no priviliges, is currently not
working. Needs change of permission gen logic also.
---
app/Auth/Role.php | 13 +++++++
app/Entities/Models/Entity.php | 6 ++-
resources/icons/groups.svg | 1 +
resources/js/components/entity-permissions.js | 24 ++++++++++++
resources/js/components/index.js | 2 +
resources/sass/_components.scss | 3 --
resources/sass/_forms.scss | 9 +++++
.../views/form/custom-checkbox.blade.php | 2 +-
.../form/entity-permissions-row.blade.php | 38 ++++++++++++++-----
.../views/form/entity-permissions.blade.php | 6 ++-
10 files changed, 87 insertions(+), 17 deletions(-)
create mode 100644 resources/icons/groups.svg
create mode 100644 resources/js/components/entity-permissions.js
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index 51b2ce301..3ae469b59 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -120,6 +120,19 @@ class Role extends Model implements Loggable
->get();
}
+ /**
+ * Get a role to represent the case of 'Everyone else' in the system.
+ * Used within the interface since the default-fallback for permissions uses role_id=0.
+ */
+ public static function getEveryoneElseRole(): self
+ {
+ return (new static())->forceFill([
+ 'id' => 0,
+ 'display_name' => 'Everyone Else',
+ 'description' => 'Set permissions for all roles not specifically overridden.'
+ ]);
+ }
+
/**
* {@inheritdoc}
*/
diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php
index 26a52073e..3528eaf2b 100644
--- a/app/Entities/Models/Entity.php
+++ b/app/Entities/Models/Entity.php
@@ -184,8 +184,10 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
*/
public function hasRestriction(int $role_id, string $action): bool
{
- return $this->permissions()->where('role_id', '=', $role_id)
- ->where('action', '=', $action)->count() > 0;
+ return $this->permissions()
+ ->where('role_id', '=', $role_id)
+ ->where('action', '=', $action)
+ ->count() > 0;
}
/**
diff --git a/resources/icons/groups.svg b/resources/icons/groups.svg
new file mode 100644
index 000000000..c99a6b503
--- /dev/null
+++ b/resources/icons/groups.svg
@@ -0,0 +1 @@
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24" width="24px"><g><path d="M12,12.75c1.63,0,3.07,0.39,4.24,0.9c1.08,0.48,1.76,1.56,1.76,2.73L18,17c0,0.55-0.45,1-1,1H7c-0.55,0-1-0.45-1-1l0-0.61 c0-1.18,0.68-2.26,1.76-2.73C8.93,13.14,10.37,12.75,12,12.75z M4,13c1.1,0,2-0.9,2-2c0-1.1-0.9-2-2-2s-2,0.9-2,2 C2,12.1,2.9,13,4,13z M5.13,14.1C4.76,14.04,4.39,14,4,14c-0.99,0-1.93,0.21-2.78,0.58C0.48,14.9,0,15.62,0,16.43L0,17 c0,0.55,0.45,1,1,1l3.5,0v-1.61C4.5,15.56,4.73,14.78,5.13,14.1z M20,13c1.1,0,2-0.9,2-2c0-1.1-0.9-2-2-2s-2,0.9-2,2 C18,12.1,18.9,13,20,13z M24,16.43c0-0.81-0.48-1.53-1.22-1.85C21.93,14.21,20.99,14,20,14c-0.39,0-0.76,0.04-1.13,0.1 c0.4,0.68,0.63,1.46,0.63,2.29V18l3.5,0c0.55,0,1-0.45,1-1L24,16.43z M12,6c1.66,0,3,1.34,3,3c0,1.66-1.34,3-3,3s-3-1.34-3-3 C9,7.34,10.34,6,12,6z"/></g></svg>
\ No newline at end of file
diff --git a/resources/js/components/entity-permissions.js b/resources/js/components/entity-permissions.js
new file mode 100644
index 000000000..8b57d3376
--- /dev/null
+++ b/resources/js/components/entity-permissions.js
@@ -0,0 +1,24 @@
+
+
+class EntityPermissions {
+
+ setup() {
+ this.everyoneInheritToggle = this.$refs.everyoneInherit;
+
+ this.setupListeners();
+ }
+
+ setupListeners() {
+ this.everyoneInheritToggle.addEventListener('change', event => {
+ const inherit = event.target.checked;
+ const permissions = document.querySelectorAll('input[type="checkbox"][name^="restrictions[0]["]');
+ for (const permission of permissions) {
+ permission.disabled = inherit;
+ permission.checked = false;
+ }
+ })
+ }
+
+}
+
+export default EntityPermissions;
\ No newline at end of file
diff --git a/resources/js/components/index.js b/resources/js/components/index.js
index 4b17dc403..7d00cb671 100644
--- a/resources/js/components/index.js
+++ b/resources/js/components/index.js
@@ -18,6 +18,7 @@ import dropdown from "./dropdown.js"
import dropdownSearch from "./dropdown-search.js"
import dropzone from "./dropzone.js"
import editorToolbox from "./editor-toolbox.js"
+import entityPermissions from "./entity-permissions";
import entitySearch from "./entity-search.js"
import entitySelector from "./entity-selector.js"
import entitySelectorPopup from "./entity-selector-popup.js"
@@ -74,6 +75,7 @@ const componentMapping = {
"dropdown-search": dropdownSearch,
"dropzone": dropzone,
"editor-toolbox": editorToolbox,
+ "entity-permissions": entityPermissions,
"entity-search": entitySearch,
"entity-selector": entitySelector,
"entity-selector-popup": entitySelectorPopup,
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index 9f737f3be..d0aadce6e 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -824,9 +824,6 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
.content-permissions-row:hover .content-permissions-row-toggle-all {
visibility: visible;
}
-.content-permissions-row-label {
- font-weight: bold;
-}
.template-item {
cursor: pointer;
diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss
index 7025aa898..5c1c8b2e8 100644
--- a/resources/sass/_forms.scss
+++ b/resources/sass/_forms.scss
@@ -266,6 +266,15 @@ input[type=color] {
background-color: rgba(0, 0, 0, 0.05);
opacity: 0.8;
}
+ input[type=checkbox][disabled] ~ * {
+ opacity: 0.8;
+ cursor: not-allowed;
+ }
+ input[type=checkbox][disabled] ~ .custom-checkbox {
+ border-color: #999;
+ color: #999 !important;
+ background: #f2f2f2;
+ }
}
.toggle-switch-list {
.toggle-switch {
diff --git a/resources/views/form/custom-checkbox.blade.php b/resources/views/form/custom-checkbox.blade.php
index 2bf4e2232..de3ffe922 100644
--- a/resources/views/form/custom-checkbox.blade.php
+++ b/resources/views/form/custom-checkbox.blade.php
@@ -5,7 +5,7 @@ $checked
$label
--}}
<label custom-checkbox class="toggle-switch @if($errors->has($name)) text-neg @endif">
- <input type="checkbox" name="{{$name}}" value="{{ $value }}" @if($checked) checked="checked" @endif>
+ <input type="checkbox" name="{{$name}}" value="{{ $value }}" @if($checked) checked="checked" @endif @if($disabled ?? false) disabled="disabled" @endif>
<span tabindex="0" role="checkbox"
aria-checked="{{ $checked ? 'true' : 'false' }}"
class="custom-checkbox text-primary">@icon('check')</span>
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index bff7315a0..f8c1dc1c7 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -1,28 +1,46 @@
<div component="permissions-table" class="content-permissions-row flex-container-row justify-space-between wrap">
- <div class="content-permissions-row-label gap-x-m flex-container-row items-center px-l py-m flex">
- <div class="text-large" title="{{ trans('common.role') }}">
- @icon('role')
+ <div class="gap-x-m flex-container-row items-center px-l py-m flex">
+ <div class="text-large" title="{{ $role->id === 0 ? 'Everyone Else' : trans('common.role') }}">
+ @icon($role->id === 0 ? 'groups' : 'role')
</div>
- <span>{{ $role->display_name }}</span>
- <button type="button"
+ <span>
+ <strong>{{ $role->display_name }}</strong> <br>
+ <small class="text-muted">{{ $role->description }}</small>
+ </span>
+ @if($role->id !== 0)
+ <button type="button"
class="ml-auto flex-none text-small text-primary text-button hover-underline content-permissions-row-toggle-all hide-under-s"
refs="permissions-table@toggle-all"
- >{{ trans('common.toggle_all') }}</button>
+ ><strong>{{ trans('common.toggle_all') }}</strong></button>
+ @endif
</div>
+ @php
+ $inheriting = ($role->id === 0 && !$model->restricted);
+ @endphp
+ @if($role->id === 0)
+ <div class="px-l flex-container-row items-center" refs="entity-permissions@everyoneInherit">
+ @include('form.custom-checkbox', [
+ 'name' => 'entity-permissions-inherit',
+ 'label' => 'Inherit defaults',
+ 'value' => 'true',
+ 'checked' => $inheriting
+ ])
+ </div>
+ @endif
<div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view'])
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting])
</div>
<div class="px-l">
@if(!$model instanceof \BookStack\Entities\Models\Page)
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create'])
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting])
@endif
</div>
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update'])
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting])
</div>
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete'])
+ @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting])
</div>
</div>
</div>
\ No newline at end of file
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 321e2f06c..408414b76 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -1,4 +1,4 @@
-<form action="{{ $model->getUrl('/permissions') }}" method="POST">
+<form component="entity-permissions" action="{{ $model->getUrl('/permissions') }}" method="POST">
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
@@ -24,6 +24,10 @@
@endforeach
</div>
+ <div class="content-permissions mt-m mb-xl">
+ @include('form.entity-permissions-row', ['role' => \BookStack\Auth\Role::getEveryoneElseRole(), 'model' => $model])
+ </div>
+
<div class="text-right">
<a href="{{ $model->getUrl() }}" class="button outline">{{ trans('common.cancel') }}</a>
<button type="submit" class="button">{{ trans('entities.permissions_save') }}</button>
From 1df9ec96477740360fc6542beed902cc2571c6de Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 7 Oct 2022 13:12:33 +0100
Subject: [PATCH 04/19] Added proper entity permission removal on role deletion
Added test to cover.
---
app/Auth/Permissions/PermissionsRepo.php | 1 +
app/Auth/Role.php | 9 +++++++++
tests/Permissions/RolesTest.php | 23 +++++++++++++++++++++++
3 files changed, 33 insertions(+)
diff --git a/app/Auth/Permissions/PermissionsRepo.php b/app/Auth/Permissions/PermissionsRepo.php
index 2c2bedb72..6dcef7256 100644
--- a/app/Auth/Permissions/PermissionsRepo.php
+++ b/app/Auth/Permissions/PermissionsRepo.php
@@ -139,6 +139,7 @@ class PermissionsRepo
}
}
+ $role->entityPermissions()->delete();
$role->jointPermissions()->delete();
Activity::add(ActivityType::ROLE_DELETE, $role);
$role->delete();
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index 3ae469b59..d5ce5cab7 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -2,6 +2,7 @@
namespace BookStack\Auth;
+use BookStack\Auth\Permissions\EntityPermission;
use BookStack\Auth\Permissions\JointPermission;
use BookStack\Auth\Permissions\RolePermission;
use BookStack\Interfaces\Loggable;
@@ -54,6 +55,14 @@ class Role extends Model implements Loggable
return $this->belongsToMany(RolePermission::class, 'permission_role', 'role_id', 'permission_id');
}
+ /**
+ * Get the entity permissions assigned to this role.
+ */
+ public function entityPermissions(): HasMany
+ {
+ return $this->hasMany(EntityPermission::class);
+ }
+
/**
* Check if this role has a permission.
*/
diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php
index 7512c6d2f..6c2f4c0df 100644
--- a/tests/Permissions/RolesTest.php
+++ b/tests/Permissions/RolesTest.php
@@ -163,6 +163,29 @@ class RolesTest extends TestCase
$this->assertEquals($this->user->id, $roleA->users()->first()->id);
}
+ public function test_entity_permissions_are_removed_on_delete()
+ {
+ /** @var Role $roleA */
+ $roleA = Role::query()->create(['display_name' => 'Entity Permissions Delete Test']);
+ $page = $this->entities->page();
+
+ $this->entities->setPermissions($page, ['view'], [$roleA]);
+
+ $this->assertDatabaseHas('entity_permissions', [
+ 'role_id' => $roleA->id,
+ 'restrictable_id' => $page->id,
+ 'restrictable_type' => $page->getMorphClass(),
+ ]);
+
+ $this->asAdmin()->delete("/settings/roles/delete/$roleA->id");
+
+ $this->assertDatabaseMissing('entity_permissions', [
+ 'role_id' => $roleA->id,
+ 'restrictable_id' => $page->id,
+ 'restrictable_type' => $page->getMorphClass(),
+ ]);
+ }
+
public function test_image_view_notice_shown_on_role_form()
{
/** @var Role $role */
From 1d3dbd6f6e1925c08237c009e8e65b5a66194ad2 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 7 Oct 2022 15:07:09 +0100
Subject: [PATCH 05/19] Migrated entity_permissions table to new flat format
Simplifies structure and limits content count, while allowing direct
mapping of new UI intent, where we may have entries with no permissions.
Not yet updated app logic to suit.
Tested via migrating and rolling-back, then comparing export data,
across a set of custom permission entries.
---
...91406_flatten_entity_permissions_table.php | 105 ++++++++++++++++++
1 file changed, 105 insertions(+)
create mode 100644 database/migrations/2022_10_07_091406_flatten_entity_permissions_table.php
diff --git a/database/migrations/2022_10_07_091406_flatten_entity_permissions_table.php b/database/migrations/2022_10_07_091406_flatten_entity_permissions_table.php
new file mode 100644
index 000000000..468f33248
--- /dev/null
+++ b/database/migrations/2022_10_07_091406_flatten_entity_permissions_table.php
@@ -0,0 +1,105 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class FlattenEntityPermissionsTable extends Migration
+{
+ /**
+ * Run the migrations.
+ *
+ * @return void
+ */
+ public function up()
+ {
+ // Remove entries for non-existing roles (Caused by previous lack of deletion handling)
+ $roleIds = DB::table('roles')->pluck('id');
+ DB::table('entity_permissions')->whereNotIn('role_id', $roleIds)->delete();
+
+ // Create new table structure for entity_permissions
+ Schema::create('new_entity_permissions', function (Blueprint $table) {
+ $table->id();
+ $table->unsignedInteger('entity_id');
+ $table->string('entity_type', 25);
+ $table->unsignedInteger('role_id')->index();
+ $table->boolean('view')->default(0);
+ $table->boolean('create')->default(0);
+ $table->boolean('update')->default(0);
+ $table->boolean('delete')->default(0);
+
+ $table->index(['entity_id', 'entity_type']);
+ });
+
+ // Migrate existing entity_permission data into new table structure
+
+ $subSelect = function (Builder $query, string $action, string $subAlias) {
+ $sub = $query->newQuery()->select('action')->from('entity_permissions', $subAlias)
+ ->whereColumn('a.restrictable_id', '=', $subAlias . '.restrictable_id')
+ ->whereColumn('a.restrictable_type', '=', $subAlias . '.restrictable_type')
+ ->whereColumn('a.role_id', '=', $subAlias . '.role_id')
+ ->where($subAlias . '.action', '=', $action);
+ return $query->selectRaw("EXISTS({$sub->toSql()})", $sub->getBindings());
+ };
+
+ $query = DB::table('entity_permissions', 'a')->select([
+ 'restrictable_id as entity_id',
+ 'restrictable_type as entity_type',
+ 'role_id',
+ 'view' => fn(Builder $query) => $subSelect($query, 'view', 'b'),
+ 'create' => fn(Builder $query) => $subSelect($query, 'create', 'c'),
+ 'update' => fn(Builder $query) => $subSelect($query, 'update', 'd'),
+ 'delete' => fn(Builder $query) => $subSelect($query, 'delete', 'e'),
+ ])->groupBy('restrictable_id', 'restrictable_type', 'role_id');
+
+ DB::table('new_entity_permissions')->insertUsing(['entity_id', 'entity_type', 'role_id', 'view', 'create', 'update', 'delete'], $query);
+
+ // Drop old entity_permissions table and replace with new structure
+ Schema::dropIfExists('entity_permissions');
+ Schema::rename('new_entity_permissions', 'entity_permissions');
+ }
+
+ /**
+ * Reverse the migrations.
+ *
+ * @return void
+ */
+ public function down()
+ {
+ // Create old table structure for entity_permissions
+ Schema::create('old_entity_permissions', function (Blueprint $table) {
+ $table->increments('id');
+ $table->integer('restrictable_id');
+ $table->string('restrictable_type', 191);
+ $table->integer('role_id')->index();
+ $table->string('action', 191)->index();
+
+ $table->index(['restrictable_id', 'restrictable_type']);
+ });
+
+ // Convert newer data format to old data format, and insert into old database
+
+ $actionQuery = function (Builder $query, string $action) {
+ return $query->select([
+ 'entity_id as restrictable_id',
+ 'entity_type as restrictable_type',
+ 'role_id',
+ ])->selectRaw("? as action", [$action])
+ ->from('entity_permissions')
+ ->where($action, '=', true);
+ };
+
+ $query = $actionQuery(DB::query(), 'view')
+ ->union(fn(Builder $query) => $actionQuery($query, 'create'))
+ ->union(fn(Builder $query) => $actionQuery($query, 'update'))
+ ->union(fn(Builder $query) => $actionQuery($query, 'delete'));
+
+ DB::table('old_entity_permissions')->insertUsing(['restrictable_id', 'restrictable_type', 'role_id', 'action'], $query);
+
+ // Drop new entity_permissions table and replace with old structure
+ Schema::dropIfExists('entity_permissions');
+ Schema::rename('old_entity_permissions', 'entity_permissions');
+ }
+}
From aee0e16194cd9d03e4d818220d52421aac8bd15f Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 8 Oct 2022 13:52:59 +0100
Subject: [PATCH 06/19] Started code update for new entity permission format
---
app/Auth/Permissions/EntityPermission.php | 21 ++++++++++----
.../Permissions/JointPermissionBuilder.php | 10 ++++---
app/Auth/Permissions/PermissionApplicator.php | 24 +++++++++++----
app/Entities/Models/Entity.php | 4 +--
app/Entities/Repos/BookshelfRepo.php | 2 +-
app/Entities/Tools/Cloner.php | 2 +-
app/Entities/Tools/PermissionsUpdater.php | 29 ++++++++++---------
.../form/entity-permissions-row.blade.php | 9 +++---
tests/Helpers/EntityProvider.php | 12 ++++----
9 files changed, 70 insertions(+), 43 deletions(-)
diff --git a/app/Auth/Permissions/EntityPermission.php b/app/Auth/Permissions/EntityPermission.php
index 131771a38..8af5f480a 100644
--- a/app/Auth/Permissions/EntityPermission.php
+++ b/app/Auth/Permissions/EntityPermission.php
@@ -3,18 +3,29 @@
namespace BookStack\Auth\Permissions;
use BookStack\Model;
+use Illuminate\Database\Eloquent\Relations\MorphTo;
+/**
+ * @property int $id
+ * @property int $role_id
+ * @property int $entity_id
+ * @property string $entity_type
+ * @property boolean $view
+ * @property boolean $create
+ * @property boolean $update
+ * @property boolean $delete
+ */
class EntityPermission extends Model
{
- protected $fillable = ['role_id', 'action'];
+ public const PERMISSIONS = ['view', 'create', 'update', 'delete'];
+
+ protected $fillable = ['role_id', 'view', 'create', 'update', 'delete'];
public $timestamps = false;
/**
- * Get all this restriction's attached entity.
- *
- * @return \Illuminate\Database\Eloquent\Relations\MorphTo
+ * Get this restriction's attached entity.
*/
- public function restrictable()
+ public function restrictable(): MorphTo
{
return $this->morphTo('restrictable');
}
diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
index f377eef5c..01a623109 100644
--- a/app/Auth/Permissions/JointPermissionBuilder.php
+++ b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -250,10 +250,13 @@ class JointPermissionBuilder
$permissions = $this->getEntityPermissionsForEntities($entities);
// Create a mapping of explicit entity permissions
+ // TODO - Handle new format, Now getting all defined entity permissions
+ // from the above call, Need to handle entries with none, and the 'Other Roles' (role_id=0)
+ // fallback option.
$permissionMap = [];
foreach ($permissions as $permission) {
- $key = $permission->restrictable_type . ':' . $permission->restrictable_id . ':' . $permission->role_id;
- $isRestricted = $entityRestrictedMap[$permission->restrictable_type . ':' . $permission->restrictable_id];
+ $key = $permission->entity_type . ':' . $permission->entity_id . ':' . $permission->role_id;
+ $isRestricted = $entityRestrictedMap[$permission->entity_type . ':' . $permission->entity_id];
$permissionMap[$key] = $isRestricted;
}
@@ -319,11 +322,10 @@ class JointPermissionBuilder
{
$idsByType = $this->entitiesToTypeIdMap($entities);
$permissionFetch = EntityPermission::query()
- ->where('action', '=', 'view')
->where(function (Builder $query) use ($idsByType) {
foreach ($idsByType as $type => $ids) {
$query->orWhere(function (Builder $query) use ($type, $ids) {
- $query->where('restrictable_type', '=', $type)->whereIn('restrictable_id', $ids);
+ $query->where('entity_type', '=', $type)->whereIn('entity_id', $ids);
});
}
});
diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index d840ccd16..6ddb152a0 100644
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -59,6 +59,8 @@ class PermissionApplicator
*/
protected function hasEntityPermission(Entity $entity, array $userRoleIds, string $action): ?bool
{
+ $this->ensureValidEntityAction($action);
+
$adminRoleId = Role::getSystemRole('admin')->id;
if (in_array($adminRoleId, $userRoleIds)) {
return true;
@@ -81,7 +83,7 @@ class PermissionApplicator
if ($currentEntity->restricted) {
return $currentEntity->permissions()
->whereIn('role_id', $userRoleIds)
- ->where('action', '=', $action)
+ ->where($action, '=', true)
->count() > 0;
}
}
@@ -95,18 +97,16 @@ class PermissionApplicator
*/
public function checkUserHasEntityPermissionOnAny(string $action, string $entityClass = ''): bool
{
- if (strpos($action, '-') !== false) {
- throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
- }
+ $this->ensureValidEntityAction($action);
$permissionQuery = EntityPermission::query()
- ->where('action', '=', $action)
+ ->where($action, '=', true)
->whereIn('role_id', $this->getCurrentUserRoleIds());
if (!empty($entityClass)) {
/** @var Entity $entityInstance */
$entityInstance = app()->make($entityClass);
- $permissionQuery = $permissionQuery->where('restrictable_type', '=', $entityInstance->getMorphClass());
+ $permissionQuery = $permissionQuery->where('entity_type', '=', $entityInstance->getMorphClass());
}
$hasPermission = $permissionQuery->count() > 0;
@@ -255,4 +255,16 @@ class PermissionApplicator
return $this->currentUser()->roles->pluck('id')->values()->all();
}
+
+ /**
+ * Ensure the given action is a valid and expected entity action.
+ * Throws an exception if invalid otherwise does nothing.
+ * @throws InvalidArgumentException
+ */
+ protected function ensureValidEntityAction(string $action): void
+ {
+ if (!in_array($action, EntityPermission::PERMISSIONS)) {
+ throw new InvalidArgumentException('Action should be a simple entity permission action, not a role permission');
+ }
+ }
}
diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php
index 3528eaf2b..a5254875d 100644
--- a/app/Entities/Models/Entity.php
+++ b/app/Entities/Models/Entity.php
@@ -176,7 +176,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
*/
public function permissions(): MorphMany
{
- return $this->morphMany(EntityPermission::class, 'restrictable');
+ return $this->morphMany(EntityPermission::class, 'entity');
}
/**
@@ -186,7 +186,7 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
{
return $this->permissions()
->where('role_id', '=', $role_id)
- ->where('action', '=', $action)
+ ->where($action, '=', true)
->count() > 0;
}
diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php
index 1f144b1a8..556ded017 100644
--- a/app/Entities/Repos/BookshelfRepo.php
+++ b/app/Entities/Repos/BookshelfRepo.php
@@ -139,7 +139,7 @@ class BookshelfRepo
*/
public function copyDownPermissions(Bookshelf $shelf, $checkUserPermissions = true): int
{
- $shelfPermissions = $shelf->permissions()->get(['role_id', 'action'])->toArray();
+ $shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
$shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
$updatedBookCount = 0;
diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php
index 86f392e61..3662b7db3 100644
--- a/app/Entities/Tools/Cloner.php
+++ b/app/Entities/Tools/Cloner.php
@@ -123,7 +123,7 @@ class Cloner
public function copyEntityPermissions(Entity $sourceEntity, Entity $targetEntity): void
{
$targetEntity->restricted = $sourceEntity->restricted;
- $permissions = $sourceEntity->permissions()->get(['role_id', 'action'])->toArray();
+ $permissions = $sourceEntity->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
$targetEntity->permissions()->delete();
$targetEntity->permissions()->createMany($permissions);
$targetEntity->rebuildPermissions();
diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php
index c771ee4b6..a547cd0a8 100644
--- a/app/Entities/Tools/PermissionsUpdater.php
+++ b/app/Entities/Tools/PermissionsUpdater.php
@@ -3,6 +3,7 @@
namespace BookStack\Entities\Tools;
use BookStack\Actions\ActivityType;
+use BookStack\Auth\Permissions\EntityPermission;
use BookStack\Auth\User;
use BookStack\Entities\Models\Entity;
use BookStack\Facades\Activity;
@@ -16,11 +17,9 @@ class PermissionsUpdater
*/
public function updateFromPermissionsForm(Entity $entity, Request $request)
{
- $restricted = $request->get('restricted') === 'true';
- $permissions = $request->get('restrictions', null);
+ $permissions = $request->get('permissions', null);
$ownerId = $request->get('owned_by', null);
- $entity->restricted = $restricted;
$entity->permissions()->delete();
if (!is_null($permissions)) {
@@ -52,18 +51,20 @@ class PermissionsUpdater
}
/**
- * Format permissions provided from a permission form to be
- * EntityPermission data.
+ * Format permissions provided from a permission form to be EntityPermission data.
*/
- protected function formatPermissionsFromRequestToEntityPermissions(array $permissions): Collection
+ protected function formatPermissionsFromRequestToEntityPermissions(array $permissions): array
{
- return collect($permissions)->flatMap(function ($restrictions, $roleId) {
- return collect($restrictions)->keys()->map(function ($action) use ($roleId) {
- return [
- 'role_id' => $roleId,
- 'action' => strtolower($action),
- ];
- });
- });
+ $formatted = [];
+
+ foreach ($permissions as $roleId => $info) {
+ $entityPermissionData = ['role_id' => $roleId];
+ foreach (EntityPermission::PERMISSIONS as $permission) {
+ $entityPermissionData[$permission] = (($info[$permission] ?? false) === "true");
+ }
+ $formatted[] = $entityPermissionData;
+ }
+
+ return $formatted;
}
}
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index f8c1dc1c7..ce8beaec3 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -28,19 +28,20 @@
</div>
@endif
<div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
+ <input type="hidden" name="permissions[{{ $role->id }}][active]" value="true">
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting])
+ @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting])
</div>
<div class="px-l">
@if(!$model instanceof \BookStack\Entities\Models\Page)
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting])
+ @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting])
@endif
</div>
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting])
+ @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting])
</div>
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'restrictions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting])
+ @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting])
</div>
</div>
</div>
\ No newline at end of file
diff --git a/tests/Helpers/EntityProvider.php b/tests/Helpers/EntityProvider.php
index 05925909e..70678a6a5 100644
--- a/tests/Helpers/EntityProvider.php
+++ b/tests/Helpers/EntityProvider.php
@@ -2,6 +2,7 @@
namespace Tests\Helpers;
+use BookStack\Auth\Permissions\EntityPermission;
use BookStack\Auth\Role;
use BookStack\Auth\User;
use BookStack\Entities\Models\Book;
@@ -207,13 +208,12 @@ class EntityProvider
$entity->permissions()->delete();
$permissions = [];
- foreach ($actions as $action) {
- foreach ($roles as $role) {
- $permissions[] = [
- 'role_id' => $role->id,
- 'action' => strtolower($action),
- ];
+ foreach ($roles as $role) {
+ $permission = ['role_id' => $role->id];
+ foreach (EntityPermission::PERMISSIONS as $possibleAction) {
+ $permission[$possibleAction] = in_array($possibleAction, $actions);
}
+ $permissions[] = $permission;
}
$entity->permissions()->createMany($permissions);
From 3839bf6bf11ac6b4d19c2ae8f62a314a2c164251 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 8 Oct 2022 14:28:44 +0100
Subject: [PATCH 07/19] Updated joint perms. gen. to use new entity permission
format
---
.../Permissions/JointPermissionBuilder.php | 46 +++++++++----------
app/Auth/Permissions/SimpleEntityData.php | 1 -
2 files changed, 23 insertions(+), 24 deletions(-)
diff --git a/app/Auth/Permissions/JointPermissionBuilder.php b/app/Auth/Permissions/JointPermissionBuilder.php
index 01a623109..79903c027 100644
--- a/app/Auth/Permissions/JointPermissionBuilder.php
+++ b/app/Auth/Permissions/JointPermissionBuilder.php
@@ -40,7 +40,7 @@ class JointPermissionBuilder
});
// Chunk through all bookshelves
- Bookshelf::query()->withTrashed()->select(['id', 'restricted', 'owned_by'])
+ Bookshelf::query()->withTrashed()->select(['id', 'owned_by'])
->chunk(50, function (EloquentCollection $shelves) use ($roles) {
$this->createManyJointPermissions($shelves->all(), $roles);
});
@@ -92,7 +92,7 @@ class JointPermissionBuilder
});
// Chunk through all bookshelves
- Bookshelf::query()->select(['id', 'restricted', 'owned_by'])
+ Bookshelf::query()->select(['id', 'owned_by'])
->chunk(50, function ($shelves) use ($roles) {
$this->createManyJointPermissions($shelves->all(), $roles);
});
@@ -138,12 +138,11 @@ class JointPermissionBuilder
protected function bookFetchQuery(): Builder
{
return Book::query()->withTrashed()
- ->select(['id', 'restricted', 'owned_by'])->with([
+ ->select(['id', 'owned_by'])->with([
'chapters' => function ($query) {
- $query->withTrashed()->select(['id', 'restricted', 'owned_by', 'book_id']);
},
'pages' => function ($query) {
- $query->withTrashed()->select(['id', 'restricted', 'owned_by', 'book_id', 'chapter_id']);
+ $query->withTrashed()->select(['id', 'owned_by', 'book_id', 'chapter_id']);
},
]);
}
@@ -218,7 +217,6 @@ class JointPermissionBuilder
$simple = new SimpleEntityData();
$simple->id = $attrs['id'];
$simple->type = $entity->getMorphClass();
- $simple->restricted = boolval($attrs['restricted'] ?? 0);
$simple->owned_by = $attrs['owned_by'] ?? 0;
$simple->book_id = $attrs['book_id'] ?? null;
$simple->chapter_id = $attrs['chapter_id'] ?? null;
@@ -240,24 +238,14 @@ class JointPermissionBuilder
$this->readyEntityCache($entities);
$jointPermissions = [];
- // Create a mapping of entity restricted statuses
- $entityRestrictedMap = [];
- foreach ($entities as $entity) {
- $entityRestrictedMap[$entity->type . ':' . $entity->id] = $entity->restricted;
- }
-
// Fetch related entity permissions
$permissions = $this->getEntityPermissionsForEntities($entities);
// Create a mapping of explicit entity permissions
- // TODO - Handle new format, Now getting all defined entity permissions
- // from the above call, Need to handle entries with none, and the 'Other Roles' (role_id=0)
- // fallback option.
$permissionMap = [];
foreach ($permissions as $permission) {
$key = $permission->entity_type . ':' . $permission->entity_id . ':' . $permission->role_id;
- $isRestricted = $entityRestrictedMap[$permission->entity_type . ':' . $permission->entity_id];
- $permissionMap[$key] = $isRestricted;
+ $permissionMap[$key] = $permission->view;
}
// Create a mapping of role permissions
@@ -347,7 +335,7 @@ class JointPermissionBuilder
return $this->createJointPermissionDataArray($entity, $roleId, true, true);
}
- if ($entity->restricted) {
+ if ($this->entityPermissionsActiveForRole($permissionMap, $entity, $roleId)) {
$hasAccess = $this->mapHasActiveRestriction($permissionMap, $entity, $roleId);
return $this->createJointPermissionDataArray($entity, $roleId, $hasAccess, $hasAccess);
@@ -360,13 +348,14 @@ class JointPermissionBuilder
// For chapters and pages, Check if explicit permissions are set on the Book.
$book = $this->getBook($entity->book_id);
$hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $book, $roleId);
- $hasPermissiveAccessToParents = !$book->restricted;
+ $hasPermissiveAccessToParents = !$this->entityPermissionsActiveForRole($permissionMap, $book, $roleId);
// For pages with a chapter, Check if explicit permissions are set on the Chapter
if ($entity->type === 'page' && $entity->chapter_id !== 0) {
$chapter = $this->getChapter($entity->chapter_id);
- $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapter->restricted;
- if ($chapter->restricted) {
+ $chapterRestricted = $this->entityPermissionsActiveForRole($permissionMap, $chapter, $roleId);
+ $hasPermissiveAccessToParents = $hasPermissiveAccessToParents && !$chapterRestricted;
+ if ($chapterRestricted) {
$hasExplicitAccessToParents = $this->mapHasActiveRestriction($permissionMap, $chapter, $roleId);
}
}
@@ -379,14 +368,25 @@ class JointPermissionBuilder
);
}
+ /**
+ * Check if entity permissions are defined within the given map, for the given entity and role.
+ * Checks for the default `role_id=0` backup option as a fallback.
+ */
+ protected function entityPermissionsActiveForRole(array $permissionMap, SimpleEntityData $entity, int $roleId): bool
+ {
+ $keyPrefix = $entity->type . ':' . $entity->id . ':';
+ return isset($permissionMap[$keyPrefix . $roleId]) || isset($permissionMap[$keyPrefix . '0']);
+ }
+
/**
* Check for an active restriction in an entity map.
*/
protected function mapHasActiveRestriction(array $entityMap, SimpleEntityData $entity, int $roleId): bool
{
- $key = $entity->type . ':' . $entity->id . ':' . $roleId;
+ $roleKey = $entity->type . ':' . $entity->id . ':' . $roleId;
+ $defaultKey = $entity->type . ':' . $entity->id . ':0';
- return $entityMap[$key] ?? false;
+ return $entityMap[$roleKey] ?? $entityMap[$defaultKey] ?? false;
}
/**
diff --git a/app/Auth/Permissions/SimpleEntityData.php b/app/Auth/Permissions/SimpleEntityData.php
index 6ec0c4179..62f5984f8 100644
--- a/app/Auth/Permissions/SimpleEntityData.php
+++ b/app/Auth/Permissions/SimpleEntityData.php
@@ -6,7 +6,6 @@ class SimpleEntityData
{
public int $id;
public string $type;
- public bool $restricted;
public int $owned_by;
public ?int $book_id;
public ?int $chapter_id;
From 06a7f1b54ad02a9597509275f80407d667014e9f Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 8 Oct 2022 15:30:03 +0100
Subject: [PATCH 08/19] Added migration to drop entity restricted field
---
...08_104202_drop_entity_restricted_field.php | 93 +++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 database/migrations/2022_10_08_104202_drop_entity_restricted_field.php
diff --git a/database/migrations/2022_10_08_104202_drop_entity_restricted_field.php b/database/migrations/2022_10_08_104202_drop_entity_restricted_field.php
new file mode 100644
index 000000000..063f924f2
--- /dev/null
+++ b/database/migrations/2022_10_08_104202_drop_entity_restricted_field.php
@@ -0,0 +1,93 @@
+<?php
+
+use Illuminate\Database\Migrations\Migration;
+use Illuminate\Database\Query\Builder;
+use Illuminate\Database\Query\JoinClause;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Schema;
+
+class DropEntityRestrictedField extends Migration
+{
+ /**
+ * Run the migrations.
+ *
+ * @return void
+ */
+ public function up()
+ {
+ // Remove entity-permissions on non-restricted entities
+ $deleteInactiveEntityPermissions = function (string $table, string $morphClass) {
+ $permissionIds = DB::table('entity_permissions')->select('entity_permissions.id as id')
+ ->join($table, function (JoinClause $join) use ($table, $morphClass) {
+ return $join->where($table . '.restricted', '=', 0)
+ ->on($table . '.id', '=', 'entity_permissions.entity_id');
+ })->where('entity_type', '=', $morphClass)
+ ->pluck('id');
+ DB::table('entity_permissions')->whereIn('id', $permissionIds)->delete();
+ };
+ $deleteInactiveEntityPermissions('pages', 'page');
+ $deleteInactiveEntityPermissions('chapters', 'chapter');
+ $deleteInactiveEntityPermissions('books', 'book');
+ $deleteInactiveEntityPermissions('bookshelves', 'bookshelf');
+
+ // Migrate restricted=1 entries to new entity_permissions (role_id=0) entries
+ $defaultEntityPermissionGenQuery = function (Builder $query, string $table, string $morphClass) {
+ return $query->select(['id as entity_id'])
+ ->selectRaw('? as entity_type', [$morphClass])
+ ->selectRaw('? as `role_id`', [0])
+ ->selectRaw('? as `view`', [0])
+ ->selectRaw('? as `create`', [0])
+ ->selectRaw('? as `update`', [0])
+ ->selectRaw('? as `delete`', [0])
+ ->from($table)
+ ->where('restricted', '=', 1);
+ };
+
+ $query = $defaultEntityPermissionGenQuery(DB::query(), 'pages', 'page')
+ ->union(fn(Builder $query) => $defaultEntityPermissionGenQuery($query, 'books', 'book'))
+ ->union(fn(Builder $query) => $defaultEntityPermissionGenQuery($query, 'chapters', 'chapter'))
+ ->union(fn(Builder $query) => $defaultEntityPermissionGenQuery($query, 'bookshelves', 'bookshelf'));
+
+ DB::table('entity_permissions')->insertUsing(['entity_id', 'entity_type', 'role_id', 'view', 'create', 'update', 'delete'], $query);
+
+ // Drop restricted columns
+ $dropRestrictedColumn = fn(Blueprint $table) => $table->dropColumn('restricted');
+ Schema::table('pages', $dropRestrictedColumn);
+ Schema::table('chapters', $dropRestrictedColumn);
+ Schema::table('books', $dropRestrictedColumn);
+ Schema::table('bookshelves', $dropRestrictedColumn);
+ }
+
+ /**
+ * Reverse the migrations.
+ *
+ * @return void
+ */
+ public function down()
+ {
+ // Create restricted columns
+ $createRestrictedColumn = fn(Blueprint $table) => $table->boolean('restricted')->index()->default(0);
+ Schema::table('pages', $createRestrictedColumn);
+ Schema::table('chapters', $createRestrictedColumn);
+ Schema::table('books', $createRestrictedColumn);
+ Schema::table('bookshelves', $createRestrictedColumn);
+
+ // Set restrictions for entities that have a default entity permission assigned
+ // Note: Possible loss of data where default entity permissions have been configured
+ $restrictEntities = function (string $table, string $morphClass) {
+ $toRestrictIds = DB::table('entity_permissions')
+ ->where('role_id', '=', 0)
+ ->where('entity_type', '=', $morphClass)
+ ->pluck('entity_id');
+ DB::table($table)->whereIn('id', $toRestrictIds)->update(['restricted' => true]);
+ };
+ $restrictEntities('pages', 'page');
+ $restrictEntities('chapters', 'chapter');
+ $restrictEntities('books', 'book');
+ $restrictEntities('bookshelves', 'bookshelf');
+
+ // Delete default entity permissions
+ DB::table('entity_permissions')->where('role_id', '=', 0)->delete();
+ }
+}
From bf591765c1e642f144675afaa031ecd283a8673b Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 9 Oct 2022 16:36:03 +0100
Subject: [PATCH 09/19] Reorgranised permission routes into their own
controller
Also introduced helpers for getting entities by slugs since we do it in
so many places.
---
app/Console/Commands/CopyShelfPermissions.php | 13 +-
app/Entities/Models/Book.php | 9 ++
app/Entities/Models/Bookshelf.php | 9 ++
app/Entities/Models/Chapter.php | 9 ++
app/Entities/Models/Page.php | 9 ++
app/Entities/Repos/BookshelfRepo.php | 25 ---
app/Entities/Tools/PermissionsUpdater.php | 28 ++++
app/Http/Controllers/BookController.php | 31 ----
app/Http/Controllers/BookshelfController.php | 43 ------
app/Http/Controllers/ChapterController.php | 33 ----
app/Http/Controllers/PageController.php | 34 ----
.../Controllers/PermissionsController.php | 146 ++++++++++++++++++
app/Http/Controllers/ReferenceController.php | 10 +-
routes/web.php | 19 +--
14 files changed, 229 insertions(+), 189 deletions(-)
create mode 100644 app/Http/Controllers/PermissionsController.php
diff --git a/app/Console/Commands/CopyShelfPermissions.php b/app/Console/Commands/CopyShelfPermissions.php
index 32adf0683..18719ae2e 100644
--- a/app/Console/Commands/CopyShelfPermissions.php
+++ b/app/Console/Commands/CopyShelfPermissions.php
@@ -3,7 +3,7 @@
namespace BookStack\Console\Commands;
use BookStack\Entities\Models\Bookshelf;
-use BookStack\Entities\Repos\BookshelfRepo;
+use BookStack\Entities\Tools\PermissionsUpdater;
use Illuminate\Console\Command;
class CopyShelfPermissions extends Command
@@ -25,19 +25,16 @@ class CopyShelfPermissions extends Command
*/
protected $description = 'Copy shelf permissions to all child books';
- /**
- * @var BookshelfRepo
- */
- protected $bookshelfRepo;
+ protected PermissionsUpdater $permissionsUpdater;
/**
* Create a new command instance.
*
* @return void
*/
- public function __construct(BookshelfRepo $repo)
+ public function __construct(PermissionsUpdater $permissionsUpdater)
{
- $this->bookshelfRepo = $repo;
+ $this->permissionsUpdater = $permissionsUpdater;
parent::__construct();
}
@@ -80,7 +77,7 @@ class CopyShelfPermissions extends Command
}
foreach ($shelves as $shelf) {
- $this->bookshelfRepo->copyDownPermissions($shelf, false);
+ $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf, false);
$this->info('Copied permissions for shelf [' . $shelf->id . ']');
}
diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php
index bf42f2008..4ced9248e 100644
--- a/app/Entities/Models/Book.php
+++ b/app/Entities/Models/Book.php
@@ -120,4 +120,13 @@ class Book extends Entity implements HasCoverImage
return $pages->concat($chapters)->sortBy('priority')->sortByDesc('draft');
}
+
+ /**
+ * Get a visible book by its slug.
+ * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
+ */
+ public static function getBySlug(string $slug): self
+ {
+ return static::visible()->where('slug', '=', $slug)->firstOrFail();
+ }
}
diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php
index cdc6648f9..6310e616f 100644
--- a/app/Entities/Models/Bookshelf.php
+++ b/app/Entities/Models/Bookshelf.php
@@ -109,4 +109,13 @@ class Bookshelf extends Entity implements HasCoverImage
$maxOrder = $this->books()->max('order');
$this->books()->attach($book->id, ['order' => $maxOrder + 1]);
}
+
+ /**
+ * Get a visible shelf by its slug.
+ * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
+ */
+ public static function getBySlug(string $slug): self
+ {
+ return static::visible()->where('slug', '=', $slug)->firstOrFail();
+ }
}
diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php
index af4bbd8e3..53eb5091f 100644
--- a/app/Entities/Models/Chapter.php
+++ b/app/Entities/Models/Chapter.php
@@ -58,4 +58,13 @@ class Chapter extends BookChild
->orderBy('priority', 'asc')
->get();
}
+
+ /**
+ * Get a visible chapter by its book and page slugs.
+ * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
+ */
+ public static function getBySlugs(string $bookSlug, string $chapterSlug): self
+ {
+ return static::visible()->whereSlugs($bookSlug, $chapterSlug)->firstOrFail();
+ }
}
diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php
index 93729d7f2..8dd769e1c 100644
--- a/app/Entities/Models/Page.php
+++ b/app/Entities/Models/Page.php
@@ -145,4 +145,13 @@ class Page extends BookChild
return $refreshed;
}
+
+ /**
+ * Get a visible page by its book and page slugs.
+ * @throws \Illuminate\Database\Eloquent\ModelNotFoundException
+ */
+ public static function getBySlugs(string $bookSlug, string $pageSlug): self
+ {
+ return static::visible()->whereSlugs($bookSlug, $pageSlug)->firstOrFail();
+ }
}
diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php
index 556ded017..d7759deb4 100644
--- a/app/Entities/Repos/BookshelfRepo.php
+++ b/app/Entities/Repos/BookshelfRepo.php
@@ -134,31 +134,6 @@ class BookshelfRepo
$shelf->books()->sync($syncData);
}
- /**
- * Copy down the permissions of the given shelf to all child books.
- */
- public function copyDownPermissions(Bookshelf $shelf, $checkUserPermissions = true): int
- {
- $shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
- $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
- $updatedBookCount = 0;
-
- /** @var Book $book */
- foreach ($shelfBooks as $book) {
- if ($checkUserPermissions && !userCan('restrictions-manage', $book)) {
- continue;
- }
- $book->permissions()->delete();
- $book->restricted = $shelf->restricted;
- $book->permissions()->createMany($shelfPermissions);
- $book->save();
- $book->rebuildPermissions();
- $updatedBookCount++;
- }
-
- return $updatedBookCount;
- }
-
/**
* Remove a bookshelf from the system.
*
diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php
index a547cd0a8..2747def18 100644
--- a/app/Entities/Tools/PermissionsUpdater.php
+++ b/app/Entities/Tools/PermissionsUpdater.php
@@ -5,6 +5,8 @@ namespace BookStack\Entities\Tools;
use BookStack\Actions\ActivityType;
use BookStack\Auth\Permissions\EntityPermission;
use BookStack\Auth\User;
+use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Bookshelf;
use BookStack\Entities\Models\Entity;
use BookStack\Facades\Activity;
use Illuminate\Http\Request;
@@ -67,4 +69,30 @@ class PermissionsUpdater
return $formatted;
}
+
+ /**
+ * Copy down the permissions of the given shelf to all child books.
+ */
+ public function updateBookPermissionsFromShelf(Bookshelf $shelf, $checkUserPermissions = true): int
+ {
+ // TODO - Fix for new format
+ $shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
+ $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
+ $updatedBookCount = 0;
+
+ /** @var Book $book */
+ foreach ($shelfBooks as $book) {
+ if ($checkUserPermissions && !userCan('restrictions-manage', $book)) {
+ continue;
+ }
+ $book->permissions()->delete();
+ $book->restricted = $shelf->restricted;
+ $book->permissions()->createMany($shelfPermissions);
+ $book->save();
+ $book->rebuildPermissions();
+ $updatedBookCount++;
+ }
+
+ return $updatedBookCount;
+ }
}
diff --git a/app/Http/Controllers/BookController.php b/app/Http/Controllers/BookController.php
index cc2f6f534..b323ae496 100644
--- a/app/Http/Controllers/BookController.php
+++ b/app/Http/Controllers/BookController.php
@@ -10,7 +10,6 @@ use BookStack\Entities\Repos\BookRepo;
use BookStack\Entities\Tools\BookContents;
use BookStack\Entities\Tools\Cloner;
use BookStack\Entities\Tools\HierarchyTransformer;
-use BookStack\Entities\Tools\PermissionsUpdater;
use BookStack\Entities\Tools\ShelfContext;
use BookStack\Exceptions\ImageUploadException;
use BookStack\Exceptions\NotFoundException;
@@ -209,36 +208,6 @@ class BookController extends Controller
return redirect('/books');
}
- /**
- * Show the permissions view.
- */
- public function showPermissions(string $bookSlug)
- {
- $book = $this->bookRepo->getBySlug($bookSlug);
- $this->checkOwnablePermission('restrictions-manage', $book);
-
- return view('books.permissions', [
- 'book' => $book,
- ]);
- }
-
- /**
- * Set the restrictions for this book.
- *
- * @throws Throwable
- */
- public function permissions(Request $request, PermissionsUpdater $permissionsUpdater, string $bookSlug)
- {
- $book = $this->bookRepo->getBySlug($bookSlug);
- $this->checkOwnablePermission('restrictions-manage', $book);
-
- $permissionsUpdater->updateFromPermissionsForm($book, $request);
-
- $this->showSuccessNotification(trans('entities.books_permissions_updated'));
-
- return redirect($book->getUrl());
- }
-
/**
* Show the view to copy a book.
*
diff --git a/app/Http/Controllers/BookshelfController.php b/app/Http/Controllers/BookshelfController.php
index 2143b876a..3c63be631 100644
--- a/app/Http/Controllers/BookshelfController.php
+++ b/app/Http/Controllers/BookshelfController.php
@@ -6,7 +6,6 @@ use BookStack\Actions\ActivityQueries;
use BookStack\Actions\View;
use BookStack\Entities\Models\Book;
use BookStack\Entities\Repos\BookshelfRepo;
-use BookStack\Entities\Tools\PermissionsUpdater;
use BookStack\Entities\Tools\ShelfContext;
use BookStack\Exceptions\ImageUploadException;
use BookStack\Exceptions\NotFoundException;
@@ -207,46 +206,4 @@ class BookshelfController extends Controller
return redirect('/shelves');
}
-
- /**
- * Show the permissions view.
- */
- public function showPermissions(string $slug)
- {
- $shelf = $this->shelfRepo->getBySlug($slug);
- $this->checkOwnablePermission('restrictions-manage', $shelf);
-
- return view('shelves.permissions', [
- 'shelf' => $shelf,
- ]);
- }
-
- /**
- * Set the permissions for this bookshelf.
- */
- public function permissions(Request $request, PermissionsUpdater $permissionsUpdater, string $slug)
- {
- $shelf = $this->shelfRepo->getBySlug($slug);
- $this->checkOwnablePermission('restrictions-manage', $shelf);
-
- $permissionsUpdater->updateFromPermissionsForm($shelf, $request);
-
- $this->showSuccessNotification(trans('entities.shelves_permissions_updated'));
-
- return redirect($shelf->getUrl());
- }
-
- /**
- * Copy the permissions of a bookshelf to the child books.
- */
- public function copyPermissions(string $slug)
- {
- $shelf = $this->shelfRepo->getBySlug($slug);
- $this->checkOwnablePermission('restrictions-manage', $shelf);
-
- $updateCount = $this->shelfRepo->copyDownPermissions($shelf);
- $this->showSuccessNotification(trans('entities.shelves_copy_permission_success', ['count' => $updateCount]));
-
- return redirect($shelf->getUrl());
- }
}
diff --git a/app/Http/Controllers/ChapterController.php b/app/Http/Controllers/ChapterController.php
index 6695c2868..4d2bcb2f1 100644
--- a/app/Http/Controllers/ChapterController.php
+++ b/app/Http/Controllers/ChapterController.php
@@ -9,7 +9,6 @@ use BookStack\Entities\Tools\BookContents;
use BookStack\Entities\Tools\Cloner;
use BookStack\Entities\Tools\HierarchyTransformer;
use BookStack\Entities\Tools\NextPreviousContentLocator;
-use BookStack\Entities\Tools\PermissionsUpdater;
use BookStack\Exceptions\MoveOperationException;
use BookStack\Exceptions\NotFoundException;
use BookStack\Exceptions\PermissionsException;
@@ -243,38 +242,6 @@ class ChapterController extends Controller
return redirect($chapterCopy->getUrl());
}
- /**
- * Show the Restrictions view.
- *
- * @throws NotFoundException
- */
- public function showPermissions(string $bookSlug, string $chapterSlug)
- {
- $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
- $this->checkOwnablePermission('restrictions-manage', $chapter);
-
- return view('chapters.permissions', [
- 'chapter' => $chapter,
- ]);
- }
-
- /**
- * Set the restrictions for this chapter.
- *
- * @throws NotFoundException
- */
- public function permissions(Request $request, PermissionsUpdater $permissionsUpdater, string $bookSlug, string $chapterSlug)
- {
- $chapter = $this->chapterRepo->getBySlug($bookSlug, $chapterSlug);
- $this->checkOwnablePermission('restrictions-manage', $chapter);
-
- $permissionsUpdater->updateFromPermissionsForm($chapter, $request);
-
- $this->showSuccessNotification(trans('entities.chapters_permissions_success'));
-
- return redirect($chapter->getUrl());
- }
-
/**
* Convert the chapter to a book.
*/
diff --git a/app/Http/Controllers/PageController.php b/app/Http/Controllers/PageController.php
index 748468b21..9e09aed16 100644
--- a/app/Http/Controllers/PageController.php
+++ b/app/Http/Controllers/PageController.php
@@ -11,7 +11,6 @@ use BookStack\Entities\Tools\NextPreviousContentLocator;
use BookStack\Entities\Tools\PageContent;
use BookStack\Entities\Tools\PageEditActivity;
use BookStack\Entities\Tools\PageEditorData;
-use BookStack\Entities\Tools\PermissionsUpdater;
use BookStack\Exceptions\NotFoundException;
use BookStack\Exceptions\PermissionsException;
use BookStack\References\ReferenceFetcher;
@@ -452,37 +451,4 @@ class PageController extends Controller
return redirect($pageCopy->getUrl());
}
-
- /**
- * Show the Permissions view.
- *
- * @throws NotFoundException
- */
- public function showPermissions(string $bookSlug, string $pageSlug)
- {
- $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug);
- $this->checkOwnablePermission('restrictions-manage', $page);
-
- return view('pages.permissions', [
- 'page' => $page,
- ]);
- }
-
- /**
- * Set the permissions for this page.
- *
- * @throws NotFoundException
- * @throws Throwable
- */
- public function permissions(Request $request, PermissionsUpdater $permissionsUpdater, string $bookSlug, string $pageSlug)
- {
- $page = $this->pageRepo->getBySlug($bookSlug, $pageSlug);
- $this->checkOwnablePermission('restrictions-manage', $page);
-
- $permissionsUpdater->updateFromPermissionsForm($page, $request);
-
- $this->showSuccessNotification(trans('entities.pages_permissions_success'));
-
- return redirect($page->getUrl());
- }
}
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
new file mode 100644
index 000000000..92f994b00
--- /dev/null
+++ b/app/Http/Controllers/PermissionsController.php
@@ -0,0 +1,146 @@
+<?php
+
+namespace BookStack\Http\Controllers;
+
+use BookStack\Entities\Models\Book;
+use BookStack\Entities\Models\Bookshelf;
+use BookStack\Entities\Models\Chapter;
+use BookStack\Entities\Models\Page;
+use BookStack\Entities\Tools\PermissionsUpdater;
+use Illuminate\Http\Request;
+
+class PermissionsController extends Controller
+{
+ protected PermissionsUpdater $permissionsUpdater;
+
+ public function __construct(PermissionsUpdater $permissionsUpdater)
+ {
+ $this->permissionsUpdater = $permissionsUpdater;
+ }
+
+ /**
+ * Show the Permissions view for a page.
+ */
+ public function showForPage(string $bookSlug, string $pageSlug)
+ {
+ $page = Page::getBySlugs($bookSlug, $pageSlug);
+ $this->checkOwnablePermission('restrictions-manage', $page);
+
+ return view('pages.permissions', [
+ 'page' => $page,
+ ]);
+ }
+
+ /**
+ * Set the permissions for a page.
+ */
+ public function updateForPage(Request $request, string $bookSlug, string $pageSlug)
+ {
+ $page = Page::getBySlugs($bookSlug, $pageSlug);
+ $this->checkOwnablePermission('restrictions-manage', $page);
+
+ $this->permissionsUpdater->updateFromPermissionsForm($page, $request);
+
+ $this->showSuccessNotification(trans('entities.pages_permissions_success'));
+
+ return redirect($page->getUrl());
+ }
+
+ /**
+ * Show the Restrictions view for a chapter.
+ */
+ public function showForChapter(string $bookSlug, string $chapterSlug)
+ {
+ $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug);
+ $this->checkOwnablePermission('restrictions-manage', $chapter);
+
+ return view('chapters.permissions', [
+ 'chapter' => $chapter,
+ ]);
+ }
+
+ /**
+ * Set the restrictions for a chapter.
+ */
+ public function updateForChapter(Request $request, string $bookSlug, string $chapterSlug)
+ {
+ $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug);
+ $this->checkOwnablePermission('restrictions-manage', $chapter);
+
+ $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request);
+
+ $this->showSuccessNotification(trans('entities.chapters_permissions_success'));
+
+ return redirect($chapter->getUrl());
+ }
+
+ /**
+ * Show the permissions view for a book.
+ */
+ public function showForBook(string $slug)
+ {
+ $book = Book::getBySlug($slug);
+ $this->checkOwnablePermission('restrictions-manage', $book);
+
+ return view('books.permissions', [
+ 'book' => $book,
+ ]);
+ }
+
+ /**
+ * Set the restrictions for a book.
+ */
+ public function updateForBook(Request $request, string $slug)
+ {
+ $book = Book::getBySlug($slug);
+ $this->checkOwnablePermission('restrictions-manage', $book);
+
+ $this->permissionsUpdater->updateFromPermissionsForm($book, $request);
+
+ $this->showSuccessNotification(trans('entities.books_permissions_updated'));
+
+ return redirect($book->getUrl());
+ }
+
+ /**
+ * Show the permissions view for a shelf.
+ */
+ public function showForShelf(string $slug)
+ {
+ $shelf = Bookshelf::getBySlug($slug);
+ $this->checkOwnablePermission('restrictions-manage', $shelf);
+
+ return view('shelves.permissions', [
+ 'shelf' => $shelf,
+ ]);
+ }
+
+ /**
+ * Set the permissions for a shelf.
+ */
+ public function updateForShelf(Request $request, string $slug)
+ {
+ $shelf = Bookshelf::getBySlug($slug);
+ $this->checkOwnablePermission('restrictions-manage', $shelf);
+
+ $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request);
+
+ $this->showSuccessNotification(trans('entities.shelves_permissions_updated'));
+
+ return redirect($shelf->getUrl());
+ }
+
+ /**
+ * Copy the permissions of a bookshelf to the child books.
+ */
+ public function copyShelfPermissionsToBooks(string $slug)
+ {
+ $shelf = Bookshelf::getBySlug($slug);
+ $this->checkOwnablePermission('restrictions-manage', $shelf);
+
+ $updateCount = $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf);
+ $this->showSuccessNotification(trans('entities.shelves_copy_permission_success', ['count' => $updateCount]));
+
+ return redirect($shelf->getUrl());
+ }
+}
diff --git a/app/Http/Controllers/ReferenceController.php b/app/Http/Controllers/ReferenceController.php
index 1daf1818c..b9b3e0eab 100644
--- a/app/Http/Controllers/ReferenceController.php
+++ b/app/Http/Controllers/ReferenceController.php
@@ -22,8 +22,7 @@ class ReferenceController extends Controller
*/
public function page(string $bookSlug, string $pageSlug)
{
- /** @var Page $page */
- $page = Page::visible()->whereSlugs($bookSlug, $pageSlug)->firstOrFail();
+ $page = Page::getBySlugs($bookSlug, $pageSlug);
$references = $this->referenceFetcher->getPageReferencesToEntity($page);
return view('pages.references', [
@@ -37,8 +36,7 @@ class ReferenceController extends Controller
*/
public function chapter(string $bookSlug, string $chapterSlug)
{
- /** @var Chapter $chapter */
- $chapter = Chapter::visible()->whereSlugs($bookSlug, $chapterSlug)->firstOrFail();
+ $chapter = Chapter::getBySlugs($bookSlug, $chapterSlug);
$references = $this->referenceFetcher->getPageReferencesToEntity($chapter);
return view('chapters.references', [
@@ -52,7 +50,7 @@ class ReferenceController extends Controller
*/
public function book(string $slug)
{
- $book = Book::visible()->where('slug', '=', $slug)->firstOrFail();
+ $book = Book::getBySlug($slug);
$references = $this->referenceFetcher->getPageReferencesToEntity($book);
return view('books.references', [
@@ -66,7 +64,7 @@ class ReferenceController extends Controller
*/
public function shelf(string $slug)
{
- $shelf = Bookshelf::visible()->where('slug', '=', $slug)->firstOrFail();
+ $shelf = Bookshelf::getBySlug($slug);
$references = $this->referenceFetcher->getPageReferencesToEntity($shelf);
return view('shelves.references', [
diff --git a/routes/web.php b/routes/web.php
index 26d4b6f13..8ee5d0739 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -19,6 +19,7 @@ use BookStack\Http\Controllers\PageController;
use BookStack\Http\Controllers\PageExportController;
use BookStack\Http\Controllers\PageRevisionController;
use BookStack\Http\Controllers\PageTemplateController;
+use BookStack\Http\Controllers\PermissionsController;
use BookStack\Http\Controllers\RecycleBinController;
use BookStack\Http\Controllers\ReferenceController;
use BookStack\Http\Controllers\RoleController;
@@ -61,9 +62,9 @@ Route::middleware('auth')->group(function () {
Route::get('/shelves/{slug}', [BookshelfController::class, 'show']);
Route::put('/shelves/{slug}', [BookshelfController::class, 'update']);
Route::delete('/shelves/{slug}', [BookshelfController::class, 'destroy']);
- Route::get('/shelves/{slug}/permissions', [BookshelfController::class, 'showPermissions']);
- Route::put('/shelves/{slug}/permissions', [BookshelfController::class, 'permissions']);
- Route::post('/shelves/{slug}/copy-permissions', [BookshelfController::class, 'copyPermissions']);
+ Route::get('/shelves/{slug}/permissions', [PermissionsController::class, 'showForShelf']);
+ Route::put('/shelves/{slug}/permissions', [PermissionsController::class, 'updateForShelf']);
+ Route::post('/shelves/{slug}/copy-permissions', [PermissionsController::class, 'copyShelfPermissionsToBooks']);
Route::get('/shelves/{slug}/references', [ReferenceController::class, 'shelf']);
// Book Creation
@@ -79,8 +80,8 @@ Route::middleware('auth')->group(function () {
Route::delete('/books/{id}', [BookController::class, 'destroy']);
Route::get('/books/{slug}/sort-item', [BookSortController::class, 'showItem']);
Route::get('/books/{slug}', [BookController::class, 'show']);
- Route::get('/books/{bookSlug}/permissions', [BookController::class, 'showPermissions']);
- Route::put('/books/{bookSlug}/permissions', [BookController::class, 'permissions']);
+ Route::get('/books/{bookSlug}/permissions', [PermissionsController::class, 'showForBook']);
+ Route::put('/books/{bookSlug}/permissions', [PermissionsController::class, 'updateForBook']);
Route::get('/books/{slug}/delete', [BookController::class, 'showDelete']);
Route::get('/books/{bookSlug}/copy', [BookController::class, 'showCopy']);
Route::post('/books/{bookSlug}/copy', [BookController::class, 'copy']);
@@ -111,8 +112,8 @@ Route::middleware('auth')->group(function () {
Route::post('/books/{bookSlug}/page/{pageSlug}/copy', [PageController::class, 'copy']);
Route::get('/books/{bookSlug}/page/{pageSlug}/delete', [PageController::class, 'showDelete']);
Route::get('/books/{bookSlug}/draft/{pageId}/delete', [PageController::class, 'showDeleteDraft']);
- Route::get('/books/{bookSlug}/page/{pageSlug}/permissions', [PageController::class, 'showPermissions']);
- Route::put('/books/{bookSlug}/page/{pageSlug}/permissions', [PageController::class, 'permissions']);
+ Route::get('/books/{bookSlug}/page/{pageSlug}/permissions', [PermissionsController::class, 'showForPage']);
+ Route::put('/books/{bookSlug}/page/{pageSlug}/permissions', [PermissionsController::class, 'updateForPage']);
Route::get('/books/{bookSlug}/page/{pageSlug}/references', [ReferenceController::class, 'page']);
Route::put('/books/{bookSlug}/page/{pageSlug}', [PageController::class, 'update']);
Route::delete('/books/{bookSlug}/page/{pageSlug}', [PageController::class, 'destroy']);
@@ -138,12 +139,12 @@ Route::middleware('auth')->group(function () {
Route::post('/books/{bookSlug}/chapter/{chapterSlug}/copy', [ChapterController::class, 'copy']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/edit', [ChapterController::class, 'edit']);
Route::post('/books/{bookSlug}/chapter/{chapterSlug}/convert-to-book', [ChapterController::class, 'convertToBook']);
- Route::get('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [ChapterController::class, 'showPermissions']);
+ Route::get('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'showForPage']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/pdf', [ChapterExportController::class, 'pdf']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/html', [ChapterExportController::class, 'html']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/markdown', [ChapterExportController::class, 'markdown']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/plaintext', [ChapterExportController::class, 'plainText']);
- Route::put('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [ChapterController::class, 'permissions']);
+ Route::put('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'updateForPage']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/references', [ReferenceController::class, 'chapter']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/delete', [ChapterController::class, 'showDelete']);
Route::delete('/books/{bookSlug}/chapter/{chapterSlug}', [ChapterController::class, 'destroy']);
From ffd6a1002e8ed40ba7b651391ee39c9ff6b2ea1f Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sun, 9 Oct 2022 17:14:11 +0100
Subject: [PATCH 10/19] Centralised handling of permission form data to own
class
Also updates show roles on permission view to just those with
permissions applied.
Fixes rounded borders for lone permission rows.
Moves "Everyone Else" handling from role to new class.
---
app/Auth/Permissions/EntityPermission.php | 10 ++++
app/Auth/Permissions/PermissionFormData.php | 57 +++++++++++++++++++
app/Auth/Role.php | 24 --------
.../Controllers/PermissionsController.php | 5 ++
resources/sass/_components.scss | 3 +
.../views/form/entity-permissions.blade.php | 4 +-
6 files changed, 77 insertions(+), 26 deletions(-)
create mode 100644 app/Auth/Permissions/PermissionFormData.php
diff --git a/app/Auth/Permissions/EntityPermission.php b/app/Auth/Permissions/EntityPermission.php
index 8af5f480a..32ebc440d 100644
--- a/app/Auth/Permissions/EntityPermission.php
+++ b/app/Auth/Permissions/EntityPermission.php
@@ -2,7 +2,9 @@
namespace BookStack\Auth\Permissions;
+use BookStack\Auth\Role;
use BookStack\Model;
+use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\MorphTo;
/**
@@ -29,4 +31,12 @@ class EntityPermission extends Model
{
return $this->morphTo('restrictable');
}
+
+ /**
+ * Get the role assigned to this entity permission.
+ */
+ public function role(): BelongsTo
+ {
+ return $this->belongsTo(Role::class);
+ }
}
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
new file mode 100644
index 000000000..07c95c534
--- /dev/null
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -0,0 +1,57 @@
+<?php
+
+namespace BookStack\Auth\Permissions;
+
+use BookStack\Auth\Role;
+use BookStack\Entities\Models\Entity;
+
+class PermissionFormData
+{
+ protected Entity $entity;
+
+ public function __construct(Entity $entity)
+ {
+ $this->entity = $entity;
+ }
+
+ /**
+ * Get the roles with permissions assigned.
+ */
+ public function rolesWithPermissions(): array
+ {
+ return $this->entity->permissions()
+ ->with('role')
+ ->where('role_id', '!=', 0)
+ ->get(['id', 'role_id'])
+ ->pluck('role')
+ ->sortBy('display_name')
+ ->all();
+ }
+
+ /**
+ * Get the roles that don't yet have specific permissions for the
+ * entity we're managing permissions for.
+ */
+ public function rolesNotAssigned(): array
+ {
+ $assigned = $this->entity->permissions()->pluck('role_id');
+ return Role::query()
+ ->where('system_name', '!=', 'admin')
+ ->whereNotIn('id', $assigned)
+ ->orderBy('display_name', 'asc')
+ ->get()
+ ->all();
+ }
+
+ /**
+ * Get the "Everyone Else" role entry.
+ */
+ public function everyoneElseRole(): Role
+ {
+ return (new Role())->forceFill([
+ 'id' => 0,
+ 'display_name' => 'Everyone Else',
+ 'description' => 'Set permissions for all roles not specifically overridden.'
+ ]);
+ }
+}
diff --git a/app/Auth/Role.php b/app/Auth/Role.php
index d5ce5cab7..17a4edcc0 100644
--- a/app/Auth/Role.php
+++ b/app/Auth/Role.php
@@ -118,30 +118,6 @@ class Role extends Model implements Loggable
return static::query()->where('hidden', '=', false)->orderBy('name')->get();
}
- /**
- * Get the roles that can be restricted.
- */
- public static function restrictable(): Collection
- {
- return static::query()
- ->where('system_name', '!=', 'admin')
- ->orderBy('display_name', 'asc')
- ->get();
- }
-
- /**
- * Get a role to represent the case of 'Everyone else' in the system.
- * Used within the interface since the default-fallback for permissions uses role_id=0.
- */
- public static function getEveryoneElseRole(): self
- {
- return (new static())->forceFill([
- 'id' => 0,
- 'display_name' => 'Everyone Else',
- 'description' => 'Set permissions for all roles not specifically overridden.'
- ]);
- }
-
/**
* {@inheritdoc}
*/
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index 92f994b00..d8dca9825 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -2,6 +2,7 @@
namespace BookStack\Http\Controllers;
+use BookStack\Auth\Permissions\PermissionFormData;
use BookStack\Entities\Models\Book;
use BookStack\Entities\Models\Bookshelf;
use BookStack\Entities\Models\Chapter;
@@ -28,6 +29,7 @@ class PermissionsController extends Controller
return view('pages.permissions', [
'page' => $page,
+ 'data' => new PermissionFormData($page),
]);
}
@@ -56,6 +58,7 @@ class PermissionsController extends Controller
return view('chapters.permissions', [
'chapter' => $chapter,
+ 'data' => new PermissionFormData($chapter),
]);
}
@@ -84,6 +87,7 @@ class PermissionsController extends Controller
return view('books.permissions', [
'book' => $book,
+ 'data' => new PermissionFormData($book),
]);
}
@@ -112,6 +116,7 @@ class PermissionsController extends Controller
return view('shelves.permissions', [
'shelf' => $shelf,
+ 'data' => new PermissionFormData($shelf),
]);
}
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index d0aadce6e..42477982a 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -818,6 +818,9 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
border-radius: 0 0 4px 4px;
border-bottom-width: 1.5px;
}
+.content-permissions-row:first-child:last-child {
+ border-radius: 4px;
+}
.content-permissions-row-toggle-all {
visibility: hidden;
}
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 408414b76..2fd0a4a43 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -19,13 +19,13 @@
@endif
<div class="content-permissions mt-m mb-xl">
- @foreach(\BookStack\Auth\Role::restrictable() as $role)
+ @foreach($data->rolesWithPermissions() as $role)
@include('form.entity-permissions-row', ['role' => $role, 'model' => $model])
@endforeach
</div>
<div class="content-permissions mt-m mb-xl">
- @include('form.entity-permissions-row', ['role' => \BookStack\Auth\Role::getEveryoneElseRole(), 'model' => $model])
+ @include('form.entity-permissions-row', ['role' => $data->everyoneElseRole(), 'model' => $model])
</div>
<div class="text-right">
From 803934d020711ee7dd01ad154cd7324806c1a098 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 10 Oct 2022 12:24:23 +0100
Subject: [PATCH 11/19] Added interface for adding/removing roles in entity
perms.
---
app/Auth/Permissions/PermissionFormData.php | 9 ++-
.../Controllers/PermissionsController.php | 18 ++++++
resources/js/components/entity-permissions.js | 62 +++++++++++++++++-
resources/sass/_buttons.scss | 13 +++-
.../form/entity-permissions-row.blade.php | 64 +++++++++++++++----
.../views/form/entity-permissions.blade.php | 33 ++++++++--
routes/web.php | 3 +
7 files changed, 177 insertions(+), 25 deletions(-)
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
index 07c95c534..781209043 100644
--- a/app/Auth/Permissions/PermissionFormData.php
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -15,16 +15,15 @@ class PermissionFormData
}
/**
- * Get the roles with permissions assigned.
+ * Get the permissions with assigned roles.
*/
- public function rolesWithPermissions(): array
+ public function permissionsWithRoles(): array
{
return $this->entity->permissions()
->with('role')
->where('role_id', '!=', 0)
- ->get(['id', 'role_id'])
- ->pluck('role')
- ->sortBy('display_name')
+ ->get()
+ ->sortBy('role.display_name')
->all();
}
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index d8dca9825..dd6c29a8a 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -2,7 +2,9 @@
namespace BookStack\Http\Controllers;
+use BookStack\Auth\Permissions\EntityPermission;
use BookStack\Auth\Permissions\PermissionFormData;
+use BookStack\Auth\Role;
use BookStack\Entities\Models\Book;
use BookStack\Entities\Models\Bookshelf;
use BookStack\Entities\Models\Chapter;
@@ -148,4 +150,20 @@ class PermissionsController extends Controller
return redirect($shelf->getUrl());
}
+
+ /**
+ * Get an empty entity permissions form row for the given role.
+ */
+ public function formRowForRole(string $entityType, string $roleId)
+ {
+ $this->checkPermissionOr('restrictions-manage', fn() => userCan('restrictions-manage-all'));
+
+ $role = Role::query()->findOrFail($roleId);
+
+ return view('form.entity-permissions-row', [
+ 'role' => $role,
+ 'permission' => new EntityPermission(),
+ 'entityType' => $entityType,
+ ]);
+ }
}
diff --git a/resources/js/components/entity-permissions.js b/resources/js/components/entity-permissions.js
index 8b57d3376..a18fc7a97 100644
--- a/resources/js/components/entity-permissions.js
+++ b/resources/js/components/entity-permissions.js
@@ -1,14 +1,21 @@
-
-
+/**
+ * @extends {Component}
+ */
class EntityPermissions {
setup() {
+ this.container = this.$el;
+ this.entityType = this.$opts.entityType;
+
this.everyoneInheritToggle = this.$refs.everyoneInherit;
+ this.roleSelect = this.$refs.roleSelect;
+ this.roleContainer = this.$refs.roleContainer;
this.setupListeners();
}
setupListeners() {
+ // "Everyone Else" inherit toggle
this.everyoneInheritToggle.addEventListener('change', event => {
const inherit = event.target.checked;
const permissions = document.querySelectorAll('input[type="checkbox"][name^="restrictions[0]["]');
@@ -16,7 +23,56 @@ class EntityPermissions {
permission.disabled = inherit;
permission.checked = false;
}
- })
+ });
+
+ // Remove role row button click
+ this.container.addEventListener('click', event => {
+ const button = event.target.closest('button');
+ if (button && button.dataset.roleId) {
+ this.removeRowOnButtonClick(button)
+ }
+ });
+
+ // Role select change
+ this.roleSelect.addEventListener('change', event => {
+ const roleId = this.roleSelect.value;
+ if (roleId) {
+ this.addRoleRow(roleId);
+ }
+ });
+ }
+
+ async addRoleRow(roleId) {
+ this.roleSelect.disabled = true;
+
+ // Remove option from select
+ const option = this.roleSelect.querySelector(`option[value="${roleId}"]`);
+ if (option) {
+ option.remove();
+ }
+
+ // Get and insert new row
+ const resp = await window.$http.get(`/permissions/form-row/${this.entityType}/${roleId}`);
+ const wrap = document.createElement('div');
+ wrap.innerHTML = resp.data;
+ const row = wrap.children[0];
+ this.roleContainer.append(row);
+ window.components.init(row);
+
+ this.roleSelect.disabled = false;
+ }
+
+ removeRowOnButtonClick(button) {
+ const row = button.closest('.content-permissions-row');
+ const roleId = button.dataset.roleId;
+ const roleName = button.dataset.roleName;
+
+ const option = document.createElement('option');
+ option.value = roleId;
+ option.textContent = roleName;
+
+ this.roleSelect.append(option);
+ row.remove();
}
}
diff --git a/resources/sass/_buttons.scss b/resources/sass/_buttons.scss
index 714dfc42c..83d17352d 100644
--- a/resources/sass/_buttons.scss
+++ b/resources/sass/_buttons.scss
@@ -109,12 +109,23 @@ button {
display: block;
}
-.button.icon {
+.button.icon, .icon-button {
.svg-icon {
margin-inline-end: 0;
}
}
+.icon-button {
+ text-align: center;
+ border: 1px solid transparent;
+}
+.icon-button:hover {
+ background-color: rgba(0, 0, 0, 0.05);
+ border-radius: 4px;
+ border-color: #DDD;
+ cursor: pointer;
+}
+
.button.svg {
display: flex;
align-items: center;
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index ce8beaec3..2bf19db64 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -1,3 +1,9 @@
+{{--
+$role - The Role to display this row for.
+$entityType - String identifier for type of entity having permissions applied.
+$permission - The entity permission containing the permissions.
+--}}
+
<div component="permissions-table" class="content-permissions-row flex-container-row justify-space-between wrap">
<div class="gap-x-m flex-container-row items-center px-l py-m flex">
<div class="text-large" title="{{ $role->id === 0 ? 'Everyone Else' : trans('common.role') }}">
@@ -15,7 +21,8 @@
@endif
</div>
@php
- $inheriting = ($role->id === 0 && !$model->restricted);
+ // TODO
+ $inheriting = ($role->id === 0);
@endphp
@if($role->id === 0)
<div class="px-l flex-container-row items-center" refs="entity-permissions@everyoneInherit">
@@ -30,18 +37,53 @@
<div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
<input type="hidden" name="permissions[{{ $role->id }}][active]" value="true">
<div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.view'), 'action' => 'view', 'disabled' => $inheriting])
+ @include('form.custom-checkbox', [
+ 'name' => 'permissions[' . $role->id . '][view]',
+ 'label' => trans('common.view'),
+ 'value' => 'true',
+ 'checked' => $permission->view,
+ 'disabled' => $inheriting
+ ])
+ </div>
+ @if($entityType !== 'page')
+ <div class="px-l">
+ @include('form.custom-checkbox', [
+ 'name' => 'permissions[' . $role->id . '][create]',
+ 'label' => trans('common.create'),
+ 'value' => 'true',
+ 'checked' => $permission->create,
+ 'disabled' => $inheriting
+ ])
+ </div>
+ @endif
+ <div class="px-l">
+ @include('form.custom-checkbox', [
+ 'name' => 'permissions[' . $role->id . '][update]',
+ 'label' => trans('common.update'),
+ 'value' => 'true',
+ 'checked' => $permission->update,
+ 'disabled' => $inheriting
+ ])
</div>
<div class="px-l">
- @if(!$model instanceof \BookStack\Entities\Models\Page)
- @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.create'), 'action' => 'create', 'disabled' => $inheriting])
- @endif
- </div>
- <div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.update'), 'action' => 'update', 'disabled' => $inheriting])
- </div>
- <div class="px-l">
- @include('form.restriction-checkbox', ['name'=>'permissions', 'label' => trans('common.delete'), 'action' => 'delete', 'disabled' => $inheriting])
+ @include('form.custom-checkbox', [
+ 'name' => 'permissions[' . $role->id . '][delete]',
+ 'label' => trans('common.delete'),
+ 'value' => 'true',
+ 'checked' => $permission->delete,
+ 'disabled' => $inheriting
+ ])
</div>
</div>
+ @if($role->id !== 0)
+ <div class="flex-container-row items-center px-m py-s">
+ <button type="button"
+ class="text-neg p-m icon-button"
+ data-role-id="{{ $role->id }}"
+ data-role-name="{{ $role->display_name }}"
+ title="Remove Row">
+ @icon('close') <span class="hide-over-m ml-xs">Remove Row</span>
+ </button>
+ </div>
+ @endif
</div>
\ No newline at end of file
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 2fd0a4a43..c6f5a4298 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -1,4 +1,7 @@
-<form component="entity-permissions" action="{{ $model->getUrl('/permissions') }}" method="POST">
+<form component="entity-permissions"
+ option:entity-permissions:entity-type="{{ $model->getType() }}"
+ action="{{ $model->getUrl('/permissions') }}"
+ method="POST">
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
@@ -18,14 +21,34 @@
<p class="text-warn">{{ trans('entities.shelves_permissions_cascade_warning') }}</p>
@endif
- <div class="content-permissions mt-m mb-xl">
- @foreach($data->rolesWithPermissions() as $role)
- @include('form.entity-permissions-row', ['role' => $role, 'model' => $model])
+ <div refs="entity-permissions@role-container" class="content-permissions mt-m mb-m">
+ @foreach($data->permissionsWithRoles() as $permission)
+ @include('form.entity-permissions-row', [
+ 'permission' => $permission,
+ 'role' => $permission->role,
+ 'entityType' => $model->getType()
+ ])
@endforeach
</div>
+ <div class="flex-container-row justify-flex-end mb-xl">
+ <div>
+ <label for="role_select">Override permissions for role</label>
+ <select name="role_select" id="role_select" refs="entity-permissions@role-select">
+ <option value="">{{ trans('common.select') }}</option>
+ @foreach($data->rolesNotAssigned() as $role)
+ <option value="{{ $role->id }}">{{ $role->display_name }}</option>
+ @endforeach
+ </select>
+ </div>
+ </div>
+
<div class="content-permissions mt-m mb-xl">
- @include('form.entity-permissions-row', ['role' => $data->everyoneElseRole(), 'model' => $model])
+ @include('form.entity-permissions-row', [
+ 'role' => $data->everyoneElseRole(),
+ 'permission' => new \BookStack\Auth\Permissions\EntityPermission(),
+ 'entityType' => $model->getType(),
+ ])
</div>
<div class="text-right">
diff --git a/routes/web.php b/routes/web.php
index 8ee5d0739..5fdfda3f0 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -215,6 +215,9 @@ Route::middleware('auth')->group(function () {
Route::get('/', [HomeController::class, 'index']);
Route::get('/home', [HomeController::class, 'index']);
+ // Permissions
+ Route::get('/permissions/form-row/{entityType}/{roleId}', [PermissionsController::class, 'formRowForRole']);
+
// Maintenance
Route::get('/settings/maintenance', [MaintenanceController::class, 'index']);
Route::delete('/settings/maintenance/cleanup-images', [MaintenanceController::class, 'cleanupImages']);
From 63056dbef477e5c5550a70f04b0e806774a44ff1 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 10 Oct 2022 16:22:51 +0100
Subject: [PATCH 12/19] Updated restricted usage on search and entity meta
details
Also removed now unused view.
---
app/Entities/Models/Entity.php | 7 ++-----
app/Search/SearchRunner.php | 4 ++--
resources/views/books/show.blade.php | 2 +-
resources/views/chapters/show.blade.php | 4 ++--
resources/views/form/restriction-checkbox.blade.php | 13 -------------
resources/views/pages/show.blade.php | 6 +++---
resources/views/shelves/show.blade.php | 2 +-
7 files changed, 11 insertions(+), 27 deletions(-)
delete mode 100644 resources/views/form/restriction-checkbox.blade.php
diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php
index a5254875d..4c399584b 100644
--- a/app/Entities/Models/Entity.php
+++ b/app/Entities/Models/Entity.php
@@ -182,12 +182,9 @@ abstract class Entity extends Model implements Sluggable, Favouritable, Viewable
/**
* Check if this entity has a specific restriction set against it.
*/
- public function hasRestriction(int $role_id, string $action): bool
+ public function hasPermissions(): bool
{
- return $this->permissions()
- ->where('role_id', '=', $role_id)
- ->where($action, '=', true)
- ->count() > 0;
+ return $this->permissions()->count() > 0;
}
/**
diff --git a/app/Search/SearchRunner.php b/app/Search/SearchRunner.php
index e36edb06c..cc44e6125 100644
--- a/app/Search/SearchRunner.php
+++ b/app/Search/SearchRunner.php
@@ -162,7 +162,7 @@ class SearchRunner
$entityQuery = $entityModelInstance->newQuery()->scopes('visible');
if ($entityModelInstance instanceof Page) {
- $entityQuery->select(array_merge($entityModelInstance::$listAttributes, ['restricted', 'owned_by']));
+ $entityQuery->select(array_merge($entityModelInstance::$listAttributes, ['owned_by']));
} else {
$entityQuery->select(['*']);
}
@@ -447,7 +447,7 @@ class SearchRunner
protected function filterIsRestricted(EloquentBuilder $query, Entity $model, $input)
{
- $query->where('restricted', '=', true);
+ $query->whereHas('permissions');
}
protected function filterViewedByMe(EloquentBuilder $query, Entity $model, $input)
diff --git a/resources/views/books/show.blade.php b/resources/views/books/show.blade.php
index 76a4a6005..b95b69d1b 100644
--- a/resources/views/books/show.blade.php
+++ b/resources/views/books/show.blade.php
@@ -71,7 +71,7 @@
<h5>{{ trans('common.details') }}</h5>
<div class="blended-links">
@include('entities.meta', ['entity' => $book])
- @if($book->restricted)
+ @if($book->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $book))
<a href="{{ $book->getUrl('/permissions') }}" class="entity-meta-item">
diff --git a/resources/views/chapters/show.blade.php b/resources/views/chapters/show.blade.php
index 1ae2d6847..b3496eae2 100644
--- a/resources/views/chapters/show.blade.php
+++ b/resources/views/chapters/show.blade.php
@@ -69,7 +69,7 @@
<div class="blended-links">
@include('entities.meta', ['entity' => $chapter])
- @if($book->restricted)
+ @if($book->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $book))
<a href="{{ $book->getUrl('/permissions') }}" class="entity-meta-item">
@@ -85,7 +85,7 @@
</div>
@endif
- @if($chapter->restricted)
+ @if($chapter->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $chapter))
<a href="{{ $chapter->getUrl('/permissions') }}" class="entity-meta-item">
diff --git a/resources/views/form/restriction-checkbox.blade.php b/resources/views/form/restriction-checkbox.blade.php
deleted file mode 100644
index 02c477f4a..000000000
--- a/resources/views/form/restriction-checkbox.blade.php
+++ /dev/null
@@ -1,13 +0,0 @@
-{{--
-$name
-$label
-$role
-$action
-$model?
---}}
-@include('form.custom-checkbox', [
- 'name' => $name . '[' . $role->id . '][' . $action . ']',
- 'label' => $label,
- 'value' => 'true',
- 'checked' => isset($model) && $model->hasRestriction($role->id, $action)
-])
\ No newline at end of file
diff --git a/resources/views/pages/show.blade.php b/resources/views/pages/show.blade.php
index b2c57c319..c053a3f94 100644
--- a/resources/views/pages/show.blade.php
+++ b/resources/views/pages/show.blade.php
@@ -81,7 +81,7 @@
<div class="blended-links">
@include('entities.meta', ['entity' => $page])
- @if($book->restricted)
+ @if($book->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $book))
<a href="{{ $book->getUrl('/permissions') }}" class="entity-meta-item">
@@ -97,7 +97,7 @@
</div>
@endif
- @if($page->chapter && $page->chapter->restricted)
+ @if($page->chapter && $page->chapter->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $page->chapter))
<a href="{{ $page->chapter->getUrl('/permissions') }}" class="entity-meta-item">
@@ -113,7 +113,7 @@
</div>
@endif
- @if($page->restricted)
+ @if($page->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $page))
<a href="{{ $page->getUrl('/permissions') }}" class="entity-meta-item">
diff --git a/resources/views/shelves/show.blade.php b/resources/views/shelves/show.blade.php
index 306d55e54..37d288956 100644
--- a/resources/views/shelves/show.blade.php
+++ b/resources/views/shelves/show.blade.php
@@ -85,7 +85,7 @@
<h5>{{ trans('common.details') }}</h5>
<div class="blended-links">
@include('entities.meta', ['entity' => $shelf])
- @if($shelf->restricted)
+ @if($shelf->hasPermissions())
<div class="active-restriction">
@if(userCan('restrictions-manage', $shelf))
<a href="{{ $shelf->getUrl('/permissions') }}" class="entity-meta-item">
From 0f68be608dde88dee24026ae175bdf6cd068e05a Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 10 Oct 2022 16:58:26 +0100
Subject: [PATCH 13/19] Removed most usages of restricted entitiy property
---
app/Auth/Permissions/PermissionApplicator.php | 26 ++++++++++++++-----
app/Console/Commands/CopyShelfPermissions.php | 4 +--
app/Entities/Models/Book.php | 2 +-
app/Entities/Models/Bookshelf.php | 2 +-
app/Entities/Models/Chapter.php | 2 +-
app/Entities/Models/Entity.php | 1 -
app/Entities/Models/Page.php | 2 +-
app/Entities/Models/PageRevision.php | 2 +-
app/Entities/Tools/Cloner.php | 1 -
app/Entities/Tools/HierarchyTransformer.php | 2 +-
app/Entities/Tools/PermissionsUpdater.php | 5 +---
app/Http/Controllers/FavouriteController.php | 2 +-
tests/Api/AttachmentsApiTest.php | 4 +--
.../CopyShelfPermissionsCommandTest.php | 8 +++---
tests/Entity/BookShelfTest.php | 4 +--
tests/Entity/BookTest.php | 4 +--
tests/Entity/ChapterTest.php | 4 +--
tests/Entity/EntitySearchTest.php | 3 +--
tests/Entity/TagTest.php | 7 ++---
tests/Helpers/EntityProvider.php | 2 --
tests/Permissions/EntityPermissionsTest.php | 1 -
tests/Uploads/AttachmentTest.php | 6 +----
22 files changed, 42 insertions(+), 52 deletions(-)
diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 6ddb152a0..56d2092cb 100644
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -66,6 +66,8 @@ class PermissionApplicator
return true;
}
+ // The chain order here is very important due to the fact we walk up the chain
+ // in the loop below. Earlier items in the chain have higher priority.
$chain = [$entity];
if ($entity instanceof Page && $entity->chapter_id) {
$chain[] = $entity->chapter;
@@ -76,16 +78,26 @@ class PermissionApplicator
}
foreach ($chain as $currentEntity) {
- if (is_null($currentEntity->restricted)) {
- throw new InvalidArgumentException('Entity restricted field used but has not been loaded');
+ $allowedByRoleId = $currentEntity->permissions()
+ ->whereIn('role_id', [0, ...$userRoleIds])
+ ->pluck($action, 'role_id');
+
+ // Continue up the chain if no applicable entity permission overrides.
+ if (empty($allowedByRoleId)) {
+ continue;
}
- if ($currentEntity->restricted) {
- return $currentEntity->permissions()
- ->whereIn('role_id', $userRoleIds)
- ->where($action, '=', true)
- ->count() > 0;
+ // If we have user-role-specific permissions set, allow if any of those
+ // role permissions allow access.
+ $hasDefault = $allowedByRoleId->has(0);
+ if (!$hasDefault || $allowedByRoleId->count() > 1) {
+ return $allowedByRoleId->search(function (bool $allowed, int $roleId) {
+ return $roleId !== 0 && $allowed;
+ }) !== false;
}
+
+ // Otherwise, return the default "Other roles" fallback value.
+ return $allowedByRoleId->get(0);
}
return null;
diff --git a/app/Console/Commands/CopyShelfPermissions.php b/app/Console/Commands/CopyShelfPermissions.php
index 18719ae2e..ec4c875ff 100644
--- a/app/Console/Commands/CopyShelfPermissions.php
+++ b/app/Console/Commands/CopyShelfPermissions.php
@@ -66,11 +66,11 @@ class CopyShelfPermissions extends Command
return;
}
- $shelves = Bookshelf::query()->get(['id', 'restricted']);
+ $shelves = Bookshelf::query()->get(['id']);
}
if ($shelfSlug) {
- $shelves = Bookshelf::query()->where('slug', '=', $shelfSlug)->get(['id', 'restricted']);
+ $shelves = Bookshelf::query()->where('slug', '=', $shelfSlug)->get(['id']);
if ($shelves->count() === 0) {
$this->info('No shelves found with the given slug.');
}
diff --git a/app/Entities/Models/Book.php b/app/Entities/Models/Book.php
index 4ced9248e..fc4556857 100644
--- a/app/Entities/Models/Book.php
+++ b/app/Entities/Models/Book.php
@@ -28,7 +28,7 @@ class Book extends Entity implements HasCoverImage
public $searchFactor = 1.2;
protected $fillable = ['name', 'description'];
- protected $hidden = ['restricted', 'pivot', 'image_id', 'deleted_at'];
+ protected $hidden = ['pivot', 'image_id', 'deleted_at'];
/**
* Get the url for this book.
diff --git a/app/Entities/Models/Bookshelf.php b/app/Entities/Models/Bookshelf.php
index 6310e616f..ad52d9d37 100644
--- a/app/Entities/Models/Bookshelf.php
+++ b/app/Entities/Models/Bookshelf.php
@@ -17,7 +17,7 @@ class Bookshelf extends Entity implements HasCoverImage
protected $fillable = ['name', 'description', 'image_id'];
- protected $hidden = ['restricted', 'image_id', 'deleted_at'];
+ protected $hidden = ['image_id', 'deleted_at'];
/**
* Get the books in this shelf.
diff --git a/app/Entities/Models/Chapter.php b/app/Entities/Models/Chapter.php
index 53eb5091f..98889ce3f 100644
--- a/app/Entities/Models/Chapter.php
+++ b/app/Entities/Models/Chapter.php
@@ -19,7 +19,7 @@ class Chapter extends BookChild
public $searchFactor = 1.2;
protected $fillable = ['name', 'description', 'priority'];
- protected $hidden = ['restricted', 'pivot', 'deleted_at'];
+ protected $hidden = ['pivot', 'deleted_at'];
/**
* Get the pages that this chapter contains.
diff --git a/app/Entities/Models/Entity.php b/app/Entities/Models/Entity.php
index 4c399584b..8bfe69365 100644
--- a/app/Entities/Models/Entity.php
+++ b/app/Entities/Models/Entity.php
@@ -42,7 +42,6 @@ use Illuminate\Database\Eloquent\SoftDeletes;
* @property Carbon $deleted_at
* @property int $created_by
* @property int $updated_by
- * @property bool $restricted
* @property Collection $tags
*
* @method static Entity|Builder visible()
diff --git a/app/Entities/Models/Page.php b/app/Entities/Models/Page.php
index 8dd769e1c..7a60b3ada 100644
--- a/app/Entities/Models/Page.php
+++ b/app/Entities/Models/Page.php
@@ -39,7 +39,7 @@ class Page extends BookChild
public $textField = 'text';
- protected $hidden = ['html', 'markdown', 'text', 'restricted', 'pivot', 'deleted_at'];
+ protected $hidden = ['html', 'markdown', 'text', 'pivot', 'deleted_at'];
protected $casts = [
'draft' => 'boolean',
diff --git a/app/Entities/Models/PageRevision.php b/app/Entities/Models/PageRevision.php
index 6517b0080..cd22db0c8 100644
--- a/app/Entities/Models/PageRevision.php
+++ b/app/Entities/Models/PageRevision.php
@@ -31,7 +31,7 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
class PageRevision extends Model implements Loggable
{
protected $fillable = ['name', 'text', 'summary'];
- protected $hidden = ['html', 'markdown', 'restricted', 'text'];
+ protected $hidden = ['html', 'markdown', 'text'];
/**
* Get the user that created the page revision.
diff --git a/app/Entities/Tools/Cloner.php b/app/Entities/Tools/Cloner.php
index 3662b7db3..52a8f4cf0 100644
--- a/app/Entities/Tools/Cloner.php
+++ b/app/Entities/Tools/Cloner.php
@@ -122,7 +122,6 @@ class Cloner
*/
public function copyEntityPermissions(Entity $sourceEntity, Entity $targetEntity): void
{
- $targetEntity->restricted = $sourceEntity->restricted;
$permissions = $sourceEntity->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
$targetEntity->permissions()->delete();
$targetEntity->permissions()->createMany($permissions);
diff --git a/app/Entities/Tools/HierarchyTransformer.php b/app/Entities/Tools/HierarchyTransformer.php
index 50d9e2eae..43cf2390e 100644
--- a/app/Entities/Tools/HierarchyTransformer.php
+++ b/app/Entities/Tools/HierarchyTransformer.php
@@ -65,7 +65,7 @@ class HierarchyTransformer
foreach ($book->chapters as $index => $chapter) {
$newBook = $this->transformChapterToBook($chapter);
$shelfBookSyncData[$newBook->id] = ['order' => $index];
- if (!$newBook->restricted) {
+ if (!$newBook->hasPermissions()) {
$this->cloner->copyEntityPermissions($shelf, $newBook);
}
}
diff --git a/app/Entities/Tools/PermissionsUpdater.php b/app/Entities/Tools/PermissionsUpdater.php
index 2747def18..eb4eb6b48 100644
--- a/app/Entities/Tools/PermissionsUpdater.php
+++ b/app/Entities/Tools/PermissionsUpdater.php
@@ -75,9 +75,8 @@ class PermissionsUpdater
*/
public function updateBookPermissionsFromShelf(Bookshelf $shelf, $checkUserPermissions = true): int
{
- // TODO - Fix for new format
$shelfPermissions = $shelf->permissions()->get(['role_id', 'view', 'create', 'update', 'delete'])->toArray();
- $shelfBooks = $shelf->books()->get(['id', 'restricted', 'owned_by']);
+ $shelfBooks = $shelf->books()->get(['id', 'owned_by']);
$updatedBookCount = 0;
/** @var Book $book */
@@ -86,9 +85,7 @@ class PermissionsUpdater
continue;
}
$book->permissions()->delete();
- $book->restricted = $shelf->restricted;
$book->permissions()->createMany($shelfPermissions);
- $book->save();
$book->rebuildPermissions();
$updatedBookCount++;
}
diff --git a/app/Http/Controllers/FavouriteController.php b/app/Http/Controllers/FavouriteController.php
index f77b04843..e46442a64 100644
--- a/app/Http/Controllers/FavouriteController.php
+++ b/app/Http/Controllers/FavouriteController.php
@@ -87,7 +87,7 @@ class FavouriteController extends Controller
$modelInstance = $model->newQuery()
->where('id', '=', $modelInfo['id'])
- ->first(['id', 'name', 'restricted', 'owned_by']);
+ ->first(['id', 'name', 'owned_by']);
$inaccessibleEntity = ($modelInstance instanceof Entity && !userCan('view', $modelInstance));
if (is_null($modelInstance) || $inaccessibleEntity) {
diff --git a/tests/Api/AttachmentsApiTest.php b/tests/Api/AttachmentsApiTest.php
index c295f7384..4d1d3b340 100644
--- a/tests/Api/AttachmentsApiTest.php
+++ b/tests/Api/AttachmentsApiTest.php
@@ -50,9 +50,7 @@ class AttachmentsApiTest extends TestCase
],
]]);
- $page->restricted = true;
- $page->save();
- $this->entities->regenPermissions($page);
+ $this->entities->setPermissions($page, [], []);
$resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
$resp->assertJsonMissing(['data' => [
diff --git a/tests/Commands/CopyShelfPermissionsCommandTest.php b/tests/Commands/CopyShelfPermissionsCommandTest.php
index 55b710ba9..4ff4fb78b 100644
--- a/tests/Commands/CopyShelfPermissionsCommandTest.php
+++ b/tests/Commands/CopyShelfPermissionsCommandTest.php
@@ -19,7 +19,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
$shelf = $this->entities->shelf();
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+ $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -28,7 +28,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
]);
$child = $shelf->books()->first();
- $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
+ $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
$this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
@@ -40,7 +40,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
Bookshelf::query()->where('id', '!=', $shelf->id)->delete();
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+ $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -48,7 +48,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
->expectsQuestion('Permission settings for all shelves will be cascaded. Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. Are you sure you want to proceed?', 'y');
$child = $shelf->books()->first();
- $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
+ $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
$this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php
index 1e740b94e..6a0bb94d5 100644
--- a/tests/Entity/BookShelfTest.php
+++ b/tests/Entity/BookShelfTest.php
@@ -295,7 +295,7 @@ class BookShelfTest extends TestCase
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->restricted), 'Child book should not be restricted by default');
+ $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -303,7 +303,7 @@ class BookShelfTest extends TestCase
$child = $shelf->books()->first();
$resp->assertRedirect($shelf->getUrl());
- $this->assertTrue(boolval($child->restricted), 'Child book should now be restricted');
+ $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
$this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
$this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
diff --git a/tests/Entity/BookTest.php b/tests/Entity/BookTest.php
index 2914162cf..cccff3a1f 100644
--- a/tests/Entity/BookTest.php
+++ b/tests/Entity/BookTest.php
@@ -304,9 +304,7 @@ class BookTest extends TestCase
// Hide child content
/** @var BookChild $page */
foreach ($book->getDirectChildren() as $child) {
- $child->restricted = true;
- $child->save();
- $this->entities->regenPermissions($child);
+ $this->entities->setPermissions($child, [], []);
}
$this->asEditor()->post($book->getUrl('/copy'), ['name' => 'My copy book']);
diff --git a/tests/Entity/ChapterTest.php b/tests/Entity/ChapterTest.php
index afc60c20e..b726280c9 100644
--- a/tests/Entity/ChapterTest.php
+++ b/tests/Entity/ChapterTest.php
@@ -101,9 +101,7 @@ class ChapterTest extends TestCase
// Hide pages to all non-admin roles
/** @var Page $page */
foreach ($chapter->pages as $page) {
- $page->restricted = true;
- $page->save();
- $this->entities->regenPermissions($page);
+ $this->entities->setPermissions($page, [], []);
}
$this->asEditor()->post($chapter->getUrl('/copy'), [
diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php
index cdb500a45..21f5dfc03 100644
--- a/tests/Entity/EntitySearchTest.php
+++ b/tests/Entity/EntitySearchTest.php
@@ -172,8 +172,7 @@ class EntitySearchTest extends TestCase
// Restricted filter
$this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertDontSee($page->name);
- $page->restricted = true;
- $page->save();
+ $this->entities->setPermissions($page, [], []);
$this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertSee($page->name);
// Date filters
diff --git a/tests/Entity/TagTest.php b/tests/Entity/TagTest.php
index ab0627601..ed5c798a5 100644
--- a/tests/Entity/TagTest.php
+++ b/tests/Entity/TagTest.php
@@ -75,9 +75,7 @@ class TagTest extends TestCase
$this->asEditor()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson(['color', 'country']);
// Set restricted permission the page
- $page->restricted = true;
- $page->save();
- $page->rebuildPermissions();
+ $this->entities->setPermissions($page, [], []);
$this->asAdmin()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson(['color', 'country']);
$this->asEditor()->get('/ajax/tags/suggest/names?search=co')->assertSimilarJson([]);
@@ -180,8 +178,7 @@ class TagTest extends TestCase
$resp = $this->get('/tags?name=SuperCategory');
$resp->assertSee('GreatTestContent');
- $page->restricted = true;
- $this->entities->regenPermissions($page);
+ $this->entities->setPermissions($page, [], []);
$resp = $this->asEditor()->get('/tags');
$resp->assertDontSee('SuperCategory');
diff --git a/tests/Helpers/EntityProvider.php b/tests/Helpers/EntityProvider.php
index 70678a6a5..4af6957a1 100644
--- a/tests/Helpers/EntityProvider.php
+++ b/tests/Helpers/EntityProvider.php
@@ -204,7 +204,6 @@ class EntityProvider
*/
public function setPermissions(Entity $entity, array $actions = [], array $roles = []): void
{
- $entity->restricted = true;
$entity->permissions()->delete();
$permissions = [];
@@ -217,7 +216,6 @@ class EntityProvider
}
$entity->permissions()->createMany($permissions);
- $entity->save();
$entity->load('permissions');
$this->regenPermissions($entity);
}
diff --git a/tests/Permissions/EntityPermissionsTest.php b/tests/Permissions/EntityPermissionsTest.php
index 7f91e7887..e88909dba 100644
--- a/tests/Permissions/EntityPermissionsTest.php
+++ b/tests/Permissions/EntityPermissionsTest.php
@@ -376,7 +376,6 @@ class EntityPermissionsTest extends TestCase
->assertSee($title);
$this->put($modelInstance->getUrl('/permissions'), [
- 'restricted' => 'true',
'restrictions' => [
$roleId => [
$permission => 'true',
diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php
index 915a9ba4d..b6fcb8f69 100644
--- a/tests/Uploads/AttachmentTest.php
+++ b/tests/Uploads/AttachmentTest.php
@@ -253,11 +253,7 @@ class AttachmentTest extends TestCase
$this->uploadFile($fileName, $page->id);
$attachment = Attachment::orderBy('id', 'desc')->take(1)->first();
- $page->restricted = true;
- $page->permissions()->delete();
- $page->save();
- $page->rebuildPermissions();
- $page->load('jointPermissions');
+ $this->entities->setPermissions($page, [], []);
$this->actingAs($viewer);
$attachmentGet = $this->get($attachment->getUrl());
From 0fae80771312050a22949362486a8e1b6f4aed04 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 10 Oct 2022 17:22:38 +0100
Subject: [PATCH 14/19] Fixed and updated "Everyone Else" permissions handling
- Fixed inheriting control for new system.
- Tested copying shelf permissions to books.
- Added additional handling for inheriting scenario identification.
---
app/Auth/Permissions/PermissionFormData.php | 12 ++++++++++++
app/Http/Controllers/PermissionsController.php | 1 +
resources/js/components/entity-permissions.js | 2 +-
.../views/form/entity-permissions-row.blade.php | 11 +++++------
resources/views/form/entity-permissions.blade.php | 9 +++++++--
5 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
index 781209043..ae06762f4 100644
--- a/app/Auth/Permissions/PermissionFormData.php
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -42,6 +42,18 @@ class PermissionFormData
->all();
}
+ /**
+ * Get the entity permission for the "Everyone Else" option.
+ */
+ public function everyoneElseEntityPermission(): EntityPermission
+ {
+ /** @var EntityPermission $permission */
+ $permission = $this->entity->permissions()
+ ->where('role_id', '=', 0)
+ ->first();
+ return $permission ?? (new EntityPermission());
+ }
+
/**
* Get the "Everyone Else" role entry.
*/
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index dd6c29a8a..93498a2b9 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -164,6 +164,7 @@ class PermissionsController extends Controller
'role' => $role,
'permission' => new EntityPermission(),
'entityType' => $entityType,
+ 'inheriting' => false,
]);
}
}
diff --git a/resources/js/components/entity-permissions.js b/resources/js/components/entity-permissions.js
index a18fc7a97..917dcc72d 100644
--- a/resources/js/components/entity-permissions.js
+++ b/resources/js/components/entity-permissions.js
@@ -18,7 +18,7 @@ class EntityPermissions {
// "Everyone Else" inherit toggle
this.everyoneInheritToggle.addEventListener('change', event => {
const inherit = event.target.checked;
- const permissions = document.querySelectorAll('input[type="checkbox"][name^="restrictions[0]["]');
+ const permissions = document.querySelectorAll('input[name^="permissions[0]["]');
for (const permission of permissions) {
permission.disabled = inherit;
permission.checked = false;
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index 2bf19db64..a7a314bf4 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -2,6 +2,7 @@
$role - The Role to display this row for.
$entityType - String identifier for type of entity having permissions applied.
$permission - The entity permission containing the permissions.
+$inheriting - Boolean if the current row should be marked as inheriting default permissions. Used for "Everyone Else" role.
--}}
<div component="permissions-table" class="content-permissions-row flex-container-row justify-space-between wrap">
@@ -20,12 +21,8 @@ $permission - The entity permission containing the permissions.
><strong>{{ trans('common.toggle_all') }}</strong></button>
@endif
</div>
- @php
- // TODO
- $inheriting = ($role->id === 0);
- @endphp
@if($role->id === 0)
- <div class="px-l flex-container-row items-center" refs="entity-permissions@everyoneInherit">
+ <div class="px-l flex-container-row items-center" refs="entity-permissions@everyone-inherit">
@include('form.custom-checkbox', [
'name' => 'entity-permissions-inherit',
'label' => 'Inherit defaults',
@@ -35,7 +32,9 @@ $permission - The entity permission containing the permissions.
</div>
@endif
<div class="flex-container-row justify-space-between gap-x-xl wrap items-center">
- <input type="hidden" name="permissions[{{ $role->id }}][active]" value="true">
+ <input type="hidden" name="permissions[{{ $role->id }}][active]"
+ @if($inheriting) disabled="disabled" @endif
+ value="true">
<div class="px-l">
@include('form.custom-checkbox', [
'name' => 'permissions[' . $role->id . '][view]',
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index c6f5a4298..a6955d33c 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -1,3 +1,6 @@
+<?php
+ /** @var \BookStack\Auth\Permissions\PermissionFormData $data */
+?>
<form component="entity-permissions"
option:entity-permissions:entity-type="{{ $model->getType() }}"
action="{{ $model->getUrl('/permissions') }}"
@@ -26,7 +29,8 @@
@include('form.entity-permissions-row', [
'permission' => $permission,
'role' => $permission->role,
- 'entityType' => $model->getType()
+ 'entityType' => $model->getType(),
+ 'inheriting' => false,
])
@endforeach
</div>
@@ -46,8 +50,9 @@
<div class="content-permissions mt-m mb-xl">
@include('form.entity-permissions-row', [
'role' => $data->everyoneElseRole(),
- 'permission' => new \BookStack\Auth\Permissions\EntityPermission(),
+ 'permission' => $data->everyoneElseEntityPermission(),
'entityType' => $model->getType(),
+ 'inheriting' => !$model->permissions()->where('role_id', '=', 0)->exists(),
])
</div>
From 25708542ff0eacea7738d478186e8b6c966427fe Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Tue, 11 Oct 2022 15:41:21 +0100
Subject: [PATCH 15/19] Refined design and text for entity permission changes
---
resources/sass/_components.scss | 6 ++-
resources/sass/_forms.scss | 4 +-
resources/views/books/permissions.blade.php | 5 +--
.../views/chapters/permissions.blade.php | 5 +--
.../views/form/entity-permissions.blade.php | 38 ++++++++++++++-----
resources/views/pages/permissions.blade.php | 5 +--
resources/views/shelves/permissions.blade.php | 15 ++++----
routes/web.php | 4 +-
8 files changed, 51 insertions(+), 31 deletions(-)
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index 42477982a..2aba3bd74 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -880,7 +880,8 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
gap: $-s;
line-height: normal;
.svg-icon {
- height: 16px;
+ height: 26px;
+ width: 26px;
margin: 0;
}
.avatar {
@@ -902,10 +903,11 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
white-space: nowrap;
}
.dropdown-search-toggle-select-caret {
- font-size: 1.5rem;
line-height: 0;
margin-left: auto;
margin-top: -2px;
+ display: flex;
+ align-items: center;
}
.dropdown-search-dropdown {
diff --git a/resources/sass/_forms.scss b/resources/sass/_forms.scss
index 5c1c8b2e8..7e0f72355 100644
--- a/resources/sass/_forms.scss
+++ b/resources/sass/_forms.scss
@@ -207,8 +207,8 @@ select {
-moz-appearance: none;
appearance: none;
background: url("data:image/svg+xml;utf8,<svg xmlns='http://www.w3.org/2000/svg' width='100' height='100' fill='%23666666'><polygon points='0,0 100,0 50,50'/></svg>");
- background-size: 12px;
- background-position: calc(100% - 20px) 70%;
+ background-size: 10px 12px;
+ background-position: calc(100% - 20px) 64%;
background-repeat: no-repeat;
@include rtl {
diff --git a/resources/views/books/permissions.blade.php b/resources/views/books/permissions.blade.php
index d72042d42..2e43338cd 100644
--- a/resources/views/books/permissions.blade.php
+++ b/resources/views/books/permissions.blade.php
@@ -14,9 +14,8 @@
]])
</div>
- <main class="card content-wrap">
- <h1 class="list-heading">{{ trans('entities.books_permissions') }}</h1>
- @include('form.entity-permissions', ['model' => $book])
+ <main class="card content-wrap auto-height">
+ @include('form.entity-permissions', ['model' => $book, 'title' => trans('entities.books_permissions')])
</main>
</div>
diff --git a/resources/views/chapters/permissions.blade.php b/resources/views/chapters/permissions.blade.php
index 6b4e21938..acdaf0ab9 100644
--- a/resources/views/chapters/permissions.blade.php
+++ b/resources/views/chapters/permissions.blade.php
@@ -15,9 +15,8 @@
]])
</div>
- <main class="card content-wrap">
- <h1 class="list-heading">{{ trans('entities.chapters_permissions') }}</h1>
- @include('form.entity-permissions', ['model' => $chapter])
+ <main class="card content-wrap auto-height">
+ @include('form.entity-permissions', ['model' => $chapter, 'title' => trans('entities.chapters_permissions')])
</main>
</div>
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index a6955d33c..5e6503e0e 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -8,21 +8,39 @@
{!! csrf_field() !!}
<input type="hidden" name="_method" value="PUT">
- <div class="grid half left-focus v-center">
+ <div class="grid half left-focus v-end gap-m wrap">
<div>
- <p class="mb-none mt-m">{{ trans('entities.permissions_intro') }}</p>
+ <h1 class="list-heading">{{ $title }}</h1>
+{{-- <p class="mb-none mt-m">{{ trans('entities.permissions_intro') }}</p>--}}
+ <p class="text-muted mb-s">
+ Set permissions here to override the default permissions provided by user roles.
+
+ @if($model instanceof \BookStack\Entities\Models\Book)
+ <br>
+ Permissions set on books will automatically cascade to child chapters and pages, unless
+ they have their own permissions defined.
+ @endif
+
+ @if($model instanceof \BookStack\Entities\Models\Chapter)
+ <br>
+ Permissions set on chapters will automatically cascade to child pages, unless
+ they have their own permissions defined.
+ @endif
+ </p>
+
+ @if($model instanceof \BookStack\Entities\Models\Bookshelf)
+ <p class="text-warn">{{ trans('entities.shelves_permissions_cascade_warning') }}</p>
+ @endif
</div>
- <div>
- <div class="form-group">
+ <div class="flex-container-row justify-flex-end">
+ <div class="form-group mb-m">
<label for="owner">{{ trans('entities.permissions_owner') }}</label>
@include('form.user-select', ['user' => $model->ownedBy, 'name' => 'owned_by'])
</div>
</div>
</div>
- @if($model instanceof \BookStack\Entities\Models\Bookshelf)
- <p class="text-warn">{{ trans('entities.shelves_permissions_cascade_warning') }}</p>
- @endif
+ <hr>
<div refs="entity-permissions@role-container" class="content-permissions mt-m mb-m">
@foreach($data->permissionsWithRoles() as $permission)
@@ -36,8 +54,8 @@
</div>
<div class="flex-container-row justify-flex-end mb-xl">
- <div>
- <label for="role_select">Override permissions for role</label>
+ <div class="flex-container-row items-center gap-m">
+ <label for="role_select" class="m-none p-none"><span class="bold">Override permissions for role</span></label>
<select name="role_select" id="role_select" refs="entity-permissions@role-select">
<option value="">{{ trans('common.select') }}</option>
@foreach($data->rolesNotAssigned() as $role)
@@ -56,6 +74,8 @@
])
</div>
+ <hr class="mb-m">
+
<div class="text-right">
<a href="{{ $model->getUrl() }}" class="button outline">{{ trans('common.cancel') }}</a>
<button type="submit" class="button">{{ trans('entities.permissions_save') }}</button>
diff --git a/resources/views/pages/permissions.blade.php b/resources/views/pages/permissions.blade.php
index 792015e28..93e14ee0d 100644
--- a/resources/views/pages/permissions.blade.php
+++ b/resources/views/pages/permissions.blade.php
@@ -16,9 +16,8 @@
]])
</div>
- <main class="card content-wrap">
- <h1 class="list-heading">{{ trans('entities.pages_permissions') }}</h1>
- @include('form.entity-permissions', ['model' => $page])
+ <main class="card content-wrap auto-height">
+ @include('form.entity-permissions', ['model' => $page, 'title' => trans('entities.pages_permissions')])
</main>
</div>
diff --git a/resources/views/shelves/permissions.blade.php b/resources/views/shelves/permissions.blade.php
index a26325518..e79b34096 100644
--- a/resources/views/shelves/permissions.blade.php
+++ b/resources/views/shelves/permissions.blade.php
@@ -2,7 +2,7 @@
@section('body')
- <div class="container small">
+ <div class="container">
<div class="my-s">
@include('entities.breadcrumbs', ['crumbs' => [
@@ -15,14 +15,15 @@
</div>
<div class="card content-wrap auto-height">
- <h1 class="list-heading">{{ trans('entities.shelves_permissions') }}</h1>
- @include('form.entity-permissions', ['model' => $shelf])
+ @include('form.entity-permissions', ['model' => $shelf, 'title' => trans('entities.shelves_permissions')])
</div>
- <div class="card content-wrap auto-height">
- <h2 class="list-heading">{{ trans('entities.shelves_copy_permissions_to_books') }}</h2>
- <p>{{ trans('entities.shelves_copy_permissions_explain') }}</p>
- <form action="{{ $shelf->getUrl('/copy-permissions') }}" method="post" class="text-right">
+ <div class="card content-wrap auto-height flex-container-row items-center gap-x-xl wrap">
+ <div class="flex">
+ <h2 class="list-heading">{{ trans('entities.shelves_copy_permissions_to_books') }}</h2>
+ <p>{{ trans('entities.shelves_copy_permissions_explain') }}</p>
+ </div>
+ <form action="{{ $shelf->getUrl('/copy-permissions') }}" method="post" class="flex text-right">
{{ csrf_field() }}
<button class="button">{{ trans('entities.shelves_copy_permissions') }}</button>
</form>
diff --git a/routes/web.php b/routes/web.php
index 5fdfda3f0..1cffbfd7d 100644
--- a/routes/web.php
+++ b/routes/web.php
@@ -139,12 +139,12 @@ Route::middleware('auth')->group(function () {
Route::post('/books/{bookSlug}/chapter/{chapterSlug}/copy', [ChapterController::class, 'copy']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/edit', [ChapterController::class, 'edit']);
Route::post('/books/{bookSlug}/chapter/{chapterSlug}/convert-to-book', [ChapterController::class, 'convertToBook']);
- Route::get('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'showForPage']);
+ Route::get('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'showForChapter']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/pdf', [ChapterExportController::class, 'pdf']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/html', [ChapterExportController::class, 'html']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/markdown', [ChapterExportController::class, 'markdown']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/export/plaintext', [ChapterExportController::class, 'plainText']);
- Route::put('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'updateForPage']);
+ Route::put('/books/{bookSlug}/chapter/{chapterSlug}/permissions', [PermissionsController::class, 'updateForChapter']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/references', [ReferenceController::class, 'chapter']);
Route::get('/books/{bookSlug}/chapter/{chapterSlug}/delete', [ChapterController::class, 'showDelete']);
Route::delete('/books/{bookSlug}/chapter/{chapterSlug}', [ChapterController::class, 'destroy']);
From 98c6422fa63ccf654c0fb9c9c178abf9145c2ffb Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Tue, 11 Oct 2022 15:52:56 +0100
Subject: [PATCH 16/19] Extracted entity perms. text to translation files
---
app/Auth/Permissions/PermissionFormData.php | 4 ++--
app/Http/Controllers/PermissionsController.php | 4 ++++
resources/lang/en/entities.php | 7 ++++++-
.../views/form/entity-permissions-row.blade.php | 6 +++---
.../views/form/entity-permissions.blade.php | 17 +++++------------
5 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
index ae06762f4..2e2af6854 100644
--- a/app/Auth/Permissions/PermissionFormData.php
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -61,8 +61,8 @@ class PermissionFormData
{
return (new Role())->forceFill([
'id' => 0,
- 'display_name' => 'Everyone Else',
- 'description' => 'Set permissions for all roles not specifically overridden.'
+ 'display_name' => trans('entities.permissions_role_everyone_else'),
+ 'description' => trans('entities.permissions_role_everyone_else_desc'),
]);
}
}
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index 93498a2b9..9d50b834b 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -29,6 +29,7 @@ class PermissionsController extends Controller
$page = Page::getBySlugs($bookSlug, $pageSlug);
$this->checkOwnablePermission('restrictions-manage', $page);
+ $this->setPageTitle(trans('entities.pages_permissions'));
return view('pages.permissions', [
'page' => $page,
'data' => new PermissionFormData($page),
@@ -58,6 +59,7 @@ class PermissionsController extends Controller
$chapter = Chapter::getBySlugs($bookSlug, $chapterSlug);
$this->checkOwnablePermission('restrictions-manage', $chapter);
+ $this->setPageTitle(trans('entities.chapters_permissions'));
return view('chapters.permissions', [
'chapter' => $chapter,
'data' => new PermissionFormData($chapter),
@@ -87,6 +89,7 @@ class PermissionsController extends Controller
$book = Book::getBySlug($slug);
$this->checkOwnablePermission('restrictions-manage', $book);
+ $this->setPageTitle(trans('entities.books_permissions'));
return view('books.permissions', [
'book' => $book,
'data' => new PermissionFormData($book),
@@ -116,6 +119,7 @@ class PermissionsController extends Controller
$shelf = Bookshelf::getBySlug($slug);
$this->checkOwnablePermission('restrictions-manage', $shelf);
+ $this->setPageTitle(trans('entities.shelves_permissions'));
return view('shelves.permissions', [
'shelf' => $shelf,
'data' => new PermissionFormData($shelf),
diff --git a/resources/lang/en/entities.php b/resources/lang/en/entities.php
index 28ec591d7..bf6201900 100644
--- a/resources/lang/en/entities.php
+++ b/resources/lang/en/entities.php
@@ -42,9 +42,14 @@ return [
// Permissions and restrictions
'permissions' => 'Permissions',
- 'permissions_intro' => 'Once enabled, These permissions will take priority over any set role permissions.',
+ 'permissions_desc' => 'Set permissions here to override the default permissions provided by user roles.',
+ 'permissions_book_cascade' => 'Permissions set on books will automatically cascade to child chapters and pages, unless they have their own permissions defined.',
+ 'permissions_chapter_cascade' => 'Permissions set on chapters will automatically cascade to child pages, unless they have their own permissions defined.',
'permissions_save' => 'Save Permissions',
'permissions_owner' => 'Owner',
+ 'permissions_role_everyone_else' => 'Everyone Else',
+ 'permissions_role_everyone_else_desc' => 'Set permissions for all roles not specifically overridden.',
+ 'permissions_role_override' => 'Override permissions for role',
// Search
'search_results' => 'Search Results',
diff --git a/resources/views/form/entity-permissions-row.blade.php b/resources/views/form/entity-permissions-row.blade.php
index a7a314bf4..d2e6a4756 100644
--- a/resources/views/form/entity-permissions-row.blade.php
+++ b/resources/views/form/entity-permissions-row.blade.php
@@ -7,7 +7,7 @@ $inheriting - Boolean if the current row should be marked as inheriting default
<div component="permissions-table" class="content-permissions-row flex-container-row justify-space-between wrap">
<div class="gap-x-m flex-container-row items-center px-l py-m flex">
- <div class="text-large" title="{{ $role->id === 0 ? 'Everyone Else' : trans('common.role') }}">
+ <div class="text-large" title="{{ $role->id === 0 ? trans('entities.permissions_role_everyone_else') : trans('common.role') }}">
@icon($role->id === 0 ? 'groups' : 'role')
</div>
<span>
@@ -80,8 +80,8 @@ $inheriting - Boolean if the current row should be marked as inheriting default
class="text-neg p-m icon-button"
data-role-id="{{ $role->id }}"
data-role-name="{{ $role->display_name }}"
- title="Remove Row">
- @icon('close') <span class="hide-over-m ml-xs">Remove Row</span>
+ title="{{ trans('common.remove') }}">
+ @icon('close') <span class="hide-over-m ml-xs">{{ trans('common.remove') }}</span>
</button>
</div>
@endif
diff --git a/resources/views/form/entity-permissions.blade.php b/resources/views/form/entity-permissions.blade.php
index 5e6503e0e..724d0fb39 100644
--- a/resources/views/form/entity-permissions.blade.php
+++ b/resources/views/form/entity-permissions.blade.php
@@ -11,20 +11,13 @@
<div class="grid half left-focus v-end gap-m wrap">
<div>
<h1 class="list-heading">{{ $title }}</h1>
-{{-- <p class="mb-none mt-m">{{ trans('entities.permissions_intro') }}</p>--}}
<p class="text-muted mb-s">
- Set permissions here to override the default permissions provided by user roles.
+ {{ trans('entities.permissions_desc') }}
@if($model instanceof \BookStack\Entities\Models\Book)
- <br>
- Permissions set on books will automatically cascade to child chapters and pages, unless
- they have their own permissions defined.
- @endif
-
- @if($model instanceof \BookStack\Entities\Models\Chapter)
- <br>
- Permissions set on chapters will automatically cascade to child pages, unless
- they have their own permissions defined.
+ <br> {{ trans('entities.permissions_book_cascade') }}
+ @elseif($model instanceof \BookStack\Entities\Models\Chapter)
+ <br> {{ trans('entities.permissions_chapter_cascade') }}
@endif
</p>
@@ -55,7 +48,7 @@
<div class="flex-container-row justify-flex-end mb-xl">
<div class="flex-container-row items-center gap-m">
- <label for="role_select" class="m-none p-none"><span class="bold">Override permissions for role</span></label>
+ <label for="role_select" class="m-none p-none"><span class="bold">{{ trans('entities.permissions_role_override') }}</span></label>
<select name="role_select" id="role_select" refs="entity-permissions@role-select">
<option value="">{{ trans('common.select') }}</option>
@foreach($data->rolesNotAssigned() as $role)
From 7792da99cecc5b839473c3257f311ef23c1e9a2c Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 12 Oct 2022 11:27:24 +0100
Subject: [PATCH 17/19] Updated entity perms. changes for dark mode support
---
resources/sass/_buttons.scss | 7 ++++---
resources/sass/_components.scss | 5 +++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/resources/sass/_buttons.scss b/resources/sass/_buttons.scss
index 83d17352d..fb3af06e8 100644
--- a/resources/sass/_buttons.scss
+++ b/resources/sass/_buttons.scss
@@ -48,9 +48,10 @@ button {
.button.outline {
background-color: transparent;
- @include lightDark(color, #666, #aaa);
+ @include lightDark(color, #666, #AAA);
fill: currentColor;
- border: 1px solid #CCC;
+ border: 1px solid;
+ @include lightDark(border-color, #CCC, #666);
&:hover, &:focus, &:active {
border: 1px solid #CCC;
box-shadow: none;
@@ -122,7 +123,7 @@ button {
.icon-button:hover {
background-color: rgba(0, 0, 0, 0.05);
border-radius: 4px;
- border-color: #DDD;
+ @include lightDark(border-color, #DDD, #444);
cursor: pointer;
}
diff --git a/resources/sass/_components.scss b/resources/sass/_components.scss
index 2aba3bd74..9fdd5a611 100644
--- a/resources/sass/_components.scss
+++ b/resources/sass/_components.scss
@@ -802,13 +802,14 @@ body.flexbox-support #entity-selector-wrap .popup-body .form-group {
box-shadow: 0 1px 3px 0 rgba(0, 0, 0, 0.1);
}
.content-permissions-row {
- border: 1.5px solid #E2E2E2;
+ border: 1.5px solid;
+ @include lightDark(border-color, #E2E2E2, #444);
border-bottom-width: 0;
label {
padding-bottom: 0;
}
&:hover {
- background-color: #F2F2F2;
+ @include lightDark(background-color, #F2F2F2, #333);
}
}
.content-permissions-row:first-child {
From bd412ddbf934c4252d795ab487bbdfc58fc0e98c Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Wed, 12 Oct 2022 12:12:36 +0100
Subject: [PATCH 18/19] Updated test for perms. changes and fixed static issues
---
app/Auth/Permissions/PermissionApplicator.php | 2 +-
app/Auth/Permissions/PermissionFormData.php | 2 +-
.../CopyShelfPermissionsCommandTest.php | 28 ++++++++++++-------
tests/Entity/BookShelfTest.php | 12 +++++---
tests/Entity/EntitySearchTest.php | 21 +++++++-------
tests/Helpers/EntityProvider.php | 6 +++-
tests/Permissions/EntityPermissionsTest.php | 9 +++---
tests/Permissions/RolesTest.php | 8 +++---
8 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/app/Auth/Permissions/PermissionApplicator.php b/app/Auth/Permissions/PermissionApplicator.php
index 56d2092cb..af372cb74 100644
--- a/app/Auth/Permissions/PermissionApplicator.php
+++ b/app/Auth/Permissions/PermissionApplicator.php
@@ -83,7 +83,7 @@ class PermissionApplicator
->pluck($action, 'role_id');
// Continue up the chain if no applicable entity permission overrides.
- if (empty($allowedByRoleId)) {
+ if ($allowedByRoleId->isEmpty()) {
continue;
}
diff --git a/app/Auth/Permissions/PermissionFormData.php b/app/Auth/Permissions/PermissionFormData.php
index 2e2af6854..8044a3c56 100644
--- a/app/Auth/Permissions/PermissionFormData.php
+++ b/app/Auth/Permissions/PermissionFormData.php
@@ -47,7 +47,7 @@ class PermissionFormData
*/
public function everyoneElseEntityPermission(): EntityPermission
{
- /** @var EntityPermission $permission */
+ /** @var ?EntityPermission $permission */
$permission = $this->entity->permissions()
->where('role_id', '=', 0)
->first();
diff --git a/tests/Commands/CopyShelfPermissionsCommandTest.php b/tests/Commands/CopyShelfPermissionsCommandTest.php
index 4ff4fb78b..cb9a845fd 100644
--- a/tests/Commands/CopyShelfPermissionsCommandTest.php
+++ b/tests/Commands/CopyShelfPermissionsCommandTest.php
@@ -19,7 +19,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
$shelf = $this->entities->shelf();
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
+ $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -28,10 +28,14 @@ class CopyShelfPermissionsCommandTest extends TestCase
]);
$child = $shelf->books()->first();
- $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
- $this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+ $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
+ $this->assertEquals(2, $child->permissions()->count(), 'Child book should have copied permissions');
+ $this->assertDatabaseHas('entity_permissions', [
+ 'entity_type' => 'book',
+ 'entity_id' => $child->id,
+ 'role_id' => $editorRole->id,
+ 'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+ ]);
}
public function test_copy_shelf_permissions_command_using_all()
@@ -40,7 +44,7 @@ class CopyShelfPermissionsCommandTest extends TestCase
Bookshelf::query()->where('id', '!=', $shelf->id)->delete();
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
+ $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -48,9 +52,13 @@ class CopyShelfPermissionsCommandTest extends TestCase
->expectsQuestion('Permission settings for all shelves will be cascaded. Books assigned to multiple shelves will receive only the permissions of it\'s last processed shelf. Are you sure you want to proceed?', 'y');
$child = $shelf->books()->first();
- $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
- $this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+ $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
+ $this->assertEquals(2, $child->permissions()->count(), 'Child book should have copied permissions');
+ $this->assertDatabaseHas('entity_permissions', [
+ 'entity_type' => 'book',
+ 'entity_id' => $child->id,
+ 'role_id' => $editorRole->id,
+ 'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+ ]);
}
}
diff --git a/tests/Entity/BookShelfTest.php b/tests/Entity/BookShelfTest.php
index 6a0bb94d5..5d919f12b 100644
--- a/tests/Entity/BookShelfTest.php
+++ b/tests/Entity/BookShelfTest.php
@@ -295,7 +295,7 @@ class BookShelfTest extends TestCase
$child = $shelf->books()->first();
$editorRole = $this->getEditor()->roles()->first();
- $this->assertFalse(boolval($child->hasPermissions()), 'Child book should not be restricted by default');
+ $this->assertFalse($child->hasPermissions(), 'Child book should not be restricted by default');
$this->assertTrue($child->permissions()->count() === 0, 'Child book should have no permissions by default');
$this->entities->setPermissions($shelf, ['view', 'update'], [$editorRole]);
@@ -303,10 +303,14 @@ class BookShelfTest extends TestCase
$child = $shelf->books()->first();
$resp->assertRedirect($shelf->getUrl());
- $this->assertTrue(boolval($child->hasPermissions()), 'Child book should now be restricted');
+ $this->assertTrue($child->hasPermissions(), 'Child book should now be restricted');
$this->assertTrue($child->permissions()->count() === 2, 'Child book should have copied permissions');
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'view', 'role_id' => $editorRole->id]);
- $this->assertDatabaseHas('entity_permissions', ['restrictable_id' => $child->id, 'action' => 'update', 'role_id' => $editorRole->id]);
+ $this->assertDatabaseHas('entity_permissions', [
+ 'entity_type' => 'book',
+ 'entity_id' => $child->id,
+ 'role_id' => $editorRole->id,
+ 'view' => true, 'update' => true, 'create' => false, 'delete' => false,
+ ]);
}
public function test_permission_page_has_a_warning_about_no_cascading()
diff --git a/tests/Entity/EntitySearchTest.php b/tests/Entity/EntitySearchTest.php
index 21f5dfc03..51fac48b2 100644
--- a/tests/Entity/EntitySearchTest.php
+++ b/tests/Entity/EntitySearchTest.php
@@ -132,9 +132,8 @@ class EntitySearchTest extends TestCase
public function test_search_filters()
{
$page = $this->entities->newPage(['name' => 'My new test quaffleachits', 'html' => 'this is about an orange donkey danzorbhsing']);
- $this->asEditor();
- $editorId = $this->getEditor()->id;
- $editorSlug = $this->getEditor()->slug;
+ $editor = $this->getEditor();
+ $this->actingAs($editor);
// Viewed filter searches
$this->get('/search?term=' . urlencode('danzorbhsing {not_viewed_by_me}'))->assertSee($page->name);
@@ -147,22 +146,22 @@ class EntitySearchTest extends TestCase
$this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
- $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editorSlug . '}'))->assertDontSee($page->name);
- $page->created_by = $editorId;
+ $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editor->slug . '}'))->assertDontSee($page->name);
+ $page->created_by = $editor->id;
$page->save();
$this->get('/search?term=' . urlencode('danzorbhsing {created_by:me}'))->assertSee($page->name);
- $this->get('/search?term=' . urlencode('danzorbhsing {created_by: ' . $editorSlug . '}'))->assertSee($page->name);
+ $this->get('/search?term=' . urlencode('danzorbhsing {created_by: ' . $editor->slug . '}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertDontSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
- $page->updated_by = $editorId;
+ $page->updated_by = $editor->id;
$page->save();
$this->get('/search?term=' . urlencode('danzorbhsing {updated_by:me}'))->assertSee($page->name);
- $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editorSlug . '}'))->assertSee($page->name);
+ $this->get('/search?term=' . urlencode('danzorbhsing {updated_by:' . $editor->slug . '}'))->assertSee($page->name);
$this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertDontSee($page->name);
- $page->owned_by = $editorId;
+ $page->owned_by = $editor->id;
$page->save();
$this->get('/search?term=' . urlencode('danzorbhsing {owned_by:me}'))->assertSee($page->name);
- $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:' . $editorSlug . '}'))->assertSee($page->name);
+ $this->get('/search?term=' . urlencode('danzorbhsing {owned_by:' . $editor->slug . '}'))->assertSee($page->name);
// Content filters
$this->get('/search?term=' . urlencode('{in_name:danzorbhsing}'))->assertDontSee($page->name);
@@ -172,7 +171,7 @@ class EntitySearchTest extends TestCase
// Restricted filter
$this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertDontSee($page->name);
- $this->entities->setPermissions($page, [], []);
+ $this->entities->setPermissions($page, ['view'], [$editor->roles->first()]);
$this->get('/search?term=' . urlencode('danzorbhsing {is_restricted}'))->assertSee($page->name);
// Date filters
diff --git a/tests/Helpers/EntityProvider.php b/tests/Helpers/EntityProvider.php
index 4af6957a1..9e8cf0b73 100644
--- a/tests/Helpers/EntityProvider.php
+++ b/tests/Helpers/EntityProvider.php
@@ -206,7 +206,11 @@ class EntityProvider
{
$entity->permissions()->delete();
- $permissions = [];
+ $permissions = [
+ // Set default permissions to not allow actions so that only the provided role permissions are at play.
+ ['role_id' => 0, 'view' => false, 'create' => false, 'update' => false, 'delete' => false],
+ ];
+
foreach ($roles as $role) {
$permission = ['role_id' => $role->id];
foreach (EntityPermission::PERMISSIONS as $possibleAction) {
diff --git a/tests/Permissions/EntityPermissionsTest.php b/tests/Permissions/EntityPermissionsTest.php
index e88909dba..6b99ba365 100644
--- a/tests/Permissions/EntityPermissionsTest.php
+++ b/tests/Permissions/EntityPermissionsTest.php
@@ -376,19 +376,18 @@ class EntityPermissionsTest extends TestCase
->assertSee($title);
$this->put($modelInstance->getUrl('/permissions'), [
- 'restrictions' => [
+ 'permissions' => [
$roleId => [
$permission => 'true',
],
],
]);
- $this->assertDatabaseHas($modelInstance->getTable(), ['id' => $modelInstance->id, 'restricted' => true]);
$this->assertDatabaseHas('entity_permissions', [
- 'restrictable_id' => $modelInstance->id,
- 'restrictable_type' => $modelInstance->getMorphClass(),
+ 'entity_id' => $modelInstance->id,
+ 'entity_type' => $modelInstance->getMorphClass(),
'role_id' => $roleId,
- 'action' => $permission,
+ $permission => true,
]);
}
diff --git a/tests/Permissions/RolesTest.php b/tests/Permissions/RolesTest.php
index 6c2f4c0df..88d400259 100644
--- a/tests/Permissions/RolesTest.php
+++ b/tests/Permissions/RolesTest.php
@@ -173,16 +173,16 @@ class RolesTest extends TestCase
$this->assertDatabaseHas('entity_permissions', [
'role_id' => $roleA->id,
- 'restrictable_id' => $page->id,
- 'restrictable_type' => $page->getMorphClass(),
+ 'entity_id' => $page->id,
+ 'entity_type' => $page->getMorphClass(),
]);
$this->asAdmin()->delete("/settings/roles/delete/$roleA->id");
$this->assertDatabaseMissing('entity_permissions', [
'role_id' => $roleA->id,
- 'restrictable_id' => $page->id,
- 'restrictable_type' => $page->getMorphClass(),
+ 'entity_id' => $page->id,
+ 'entity_type' => $page->getMorphClass(),
]);
}
From 6951aa3d39c061fdeeefa00d94967fdd378f9a92 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Fri, 14 Oct 2022 16:03:06 +0100
Subject: [PATCH 19/19] Fixed permission row permission check
---
app/Http/Controllers/PermissionsController.php | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/Http/Controllers/PermissionsController.php b/app/Http/Controllers/PermissionsController.php
index 9d50b834b..7d908733b 100644
--- a/app/Http/Controllers/PermissionsController.php
+++ b/app/Http/Controllers/PermissionsController.php
@@ -160,7 +160,7 @@ class PermissionsController extends Controller
*/
public function formRowForRole(string $entityType, string $roleId)
{
- $this->checkPermissionOr('restrictions-manage', fn() => userCan('restrictions-manage-all'));
+ $this->checkPermissionOr('restrictions-manage-all', fn() => userCan('restrictions-manage-own'));
$role = Role::query()->findOrFail($roleId);