From add091305c0e23face6070f40f52c24e2d0f6df4 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Jul 2025 12:15:25 +0100 Subject: [PATCH 1/3] Perms: Removed entity perm regen on general update Should not be needed here as this is not directly used for information which should impact permissions. Been through uses to ensure that this is the case. --- app/Entities/Repos/BaseRepo.php | 3 +-- app/Entities/Repos/PageRepo.php | 3 ++- app/Sorting/BookSorter.php | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/app/Entities/Repos/BaseRepo.php b/app/Entities/Repos/BaseRepo.php index 151d5b055..ac5a44e67 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -77,7 +77,6 @@ class BaseRepo $entity->touch(); } - $entity->rebuildPermissions(); $entity->indexForSearch(); $this->referenceStore->updateForEntity($entity); @@ -139,7 +138,7 @@ class BaseRepo /** * Sort the parent of the given entity, if any auto sort actions are set for it. - * Typical ran during create/update/insert events. + * Typically ran during create/update/insert events. */ public function sortParent(Entity $entity): void { diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index c3be6d826..f081b33dc 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -77,6 +77,7 @@ class PageRepo $draft->priority = $this->getNewPriority($draft); $this->updateTemplateStatusAndContentFromInput($draft, $input); $this->baseRepo->update($draft, $input); + $draft->rebuildPermissions(); $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); $this->revisionRepo->storeNewForPage($draft, $summary); @@ -91,7 +92,7 @@ class PageRepo /** * Directly update the content for the given page from the provided input. * Used for direct content access in a way that performs required changes - * (Search index & reference regen) without performing an official update. + * (Search index and reference regen) without performing an official update. */ public function setContentFromInput(Page $page, array $input): void { diff --git a/app/Sorting/BookSorter.php b/app/Sorting/BookSorter.php index 6710f070a..cf41a6a94 100644 --- a/app/Sorting/BookSorter.php +++ b/app/Sorting/BookSorter.php @@ -2,7 +2,6 @@ namespace BookStack\Sorting; -use BookStack\App\Model; use BookStack\Entities\Models\Book; use BookStack\Entities\Models\BookChild; use BookStack\Entities\Models\Chapter; From 47fd578edb0df2cc1613d7a823da6becc98e1f1a Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Wed, 2 Jul 2025 22:25:59 +0100 Subject: [PATCH 2/3] Perms: Added transactions around permission effecting actions --- app/Entities/Controllers/BookController.php | 5 +- .../Controllers/ChapterController.php | 5 +- app/Entities/Repos/BookRepo.php | 26 ++++---- app/Entities/Repos/BookshelfRepo.php | 15 ++--- app/Entities/Repos/ChapterRepo.php | 31 ++++----- app/Entities/Repos/PageRepo.php | 51 ++++++++------- app/Entities/Tools/HierarchyTransformer.php | 17 ++--- app/Entities/Tools/TrashCan.php | 28 +++++---- app/Permissions/JointPermissionBuilder.php | 36 +++++------ app/Permissions/PermissionsController.php | 22 +++++-- app/Permissions/PermissionsRepo.php | 63 ++++++++++--------- app/Sorting/BookSortController.php | 19 +++--- app/Util/DatabaseTransaction.php | 42 +++++++++++++ 13 files changed, 219 insertions(+), 141 deletions(-) create mode 100644 app/Util/DatabaseTransaction.php diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index b1685081a..5d3d67f64 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\NotFoundException; use BookStack\Facades\Activity; use BookStack\Http\Controller; use BookStack\References\ReferenceFetcher; +use BookStack\Util\DatabaseTransaction; use BookStack\Util\SimpleListOptions; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; @@ -263,7 +264,9 @@ class BookController extends Controller $this->checkPermission('bookshelf-create-all'); $this->checkPermission('book-create-all'); - $shelf = $transformer->transformBookToShelf($book); + $shelf = (new DatabaseTransaction(function () use ($book, $transformer) { + return $transformer->transformBookToShelf($book); + }))->run(); return redirect($shelf->getUrl()); } diff --git a/app/Entities/Controllers/ChapterController.php b/app/Entities/Controllers/ChapterController.php index 4274589e2..677745500 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\NotifyException; use BookStack\Exceptions\PermissionsException; use BookStack\Http\Controller; use BookStack\References\ReferenceFetcher; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; use Illuminate\Validation\ValidationException; use Throwable; @@ -269,7 +270,9 @@ class ChapterController extends Controller $this->checkOwnablePermission('chapter-delete', $chapter); $this->checkPermission('book-create-all'); - $book = $transformer->transformChapterToBook($chapter); + $book = (new DatabaseTransaction(function () use ($chapter, $transformer) { + return $transformer->transformChapterToBook($chapter); + }))->run(); return redirect($book->getUrl()); } diff --git a/app/Entities/Repos/BookRepo.php b/app/Entities/Repos/BookRepo.php index 92e6a81c3..6d28d5d6a 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -10,6 +10,7 @@ use BookStack\Exceptions\ImageUploadException; use BookStack\Facades\Activity; use BookStack\Sorting\SortRule; use BookStack\Uploads\ImageRepo; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Http\UploadedFile; @@ -28,19 +29,22 @@ class BookRepo */ public function create(array $input): Book { - $book = new Book(); - $this->baseRepo->create($book, $input); - $this->baseRepo->updateCoverImage($book, $input['image'] ?? null); - $this->baseRepo->updateDefaultTemplate($book, intval($input['default_template_id'] ?? null)); - Activity::add(ActivityType::BOOK_CREATE, $book); + return (new DatabaseTransaction(function () use ($input) { + $book = new Book(); - $defaultBookSortSetting = intval(setting('sorting-book-default', '0')); - if ($defaultBookSortSetting && SortRule::query()->find($defaultBookSortSetting)) { - $book->sort_rule_id = $defaultBookSortSetting; - $book->save(); - } + $this->baseRepo->create($book, $input); + $this->baseRepo->updateCoverImage($book, $input['image'] ?? null); + $this->baseRepo->updateDefaultTemplate($book, intval($input['default_template_id'] ?? null)); + Activity::add(ActivityType::BOOK_CREATE, $book); - return $book; + $defaultBookSortSetting = intval(setting('sorting-book-default', '0')); + if ($defaultBookSortSetting && SortRule::query()->find($defaultBookSortSetting)) { + $book->sort_rule_id = $defaultBookSortSetting; + $book->save(); + } + + return $book; + }))->run(); } /** diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index a00349ef1..5d4d604bc 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -7,6 +7,7 @@ use BookStack\Entities\Models\Bookshelf; use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\TrashCan; use BookStack\Facades\Activity; +use BookStack\Util\DatabaseTransaction; use Exception; class BookshelfRepo @@ -23,13 +24,13 @@ class BookshelfRepo */ public function create(array $input, array $bookIds): Bookshelf { - $shelf = new Bookshelf(); - $this->baseRepo->create($shelf, $input); - $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); - $this->updateBooks($shelf, $bookIds); - Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); - - return $shelf; + return (new DatabaseTransaction(function () use ($input, $bookIds) { + $shelf = new Bookshelf(); + $this->baseRepo->create($shelf, $input); + $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); + $this->updateBooks($shelf, $bookIds); + Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); + }))->run(); } /** diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index fdf2de4e2..2ab6e671f 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -11,6 +11,7 @@ use BookStack\Entities\Tools\TrashCan; use BookStack\Exceptions\MoveOperationException; use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; +use BookStack\Util\DatabaseTransaction; use Exception; class ChapterRepo @@ -27,16 +28,16 @@ class ChapterRepo */ public function create(array $input, Book $parentBook): Chapter { - $chapter = new Chapter(); - $chapter->book_id = $parentBook->id; - $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; - $this->baseRepo->create($chapter, $input); - $this->baseRepo->updateDefaultTemplate($chapter, intval($input['default_template_id'] ?? null)); - Activity::add(ActivityType::CHAPTER_CREATE, $chapter); + return (new DatabaseTransaction(function () use ($input, $parentBook) { + $chapter = new Chapter(); + $chapter->book_id = $parentBook->id; + $chapter->priority = (new BookContents($parentBook))->getLastPriority() + 1; + $this->baseRepo->create($chapter, $input); + $this->baseRepo->updateDefaultTemplate($chapter, intval($input['default_template_id'] ?? null)); + Activity::add(ActivityType::CHAPTER_CREATE, $chapter); - $this->baseRepo->sortParent($chapter); - - return $chapter; + $this->baseRepo->sortParent($chapter); + }))->run(); } /** @@ -88,12 +89,14 @@ class ChapterRepo throw new PermissionsException('User does not have permission to create a chapter within the chosen book'); } - $chapter->changeBook($parent->id); - $chapter->rebuildPermissions(); - Activity::add(ActivityType::CHAPTER_MOVE, $chapter); + return (new DatabaseTransaction(function () use ($chapter, $parent) { + $chapter->changeBook($parent->id); + $chapter->rebuildPermissions(); + Activity::add(ActivityType::CHAPTER_MOVE, $chapter); - $this->baseRepo->sortParent($chapter); + $this->baseRepo->sortParent($chapter); - return $parent; + return $parent; + }))->run(); } } diff --git a/app/Entities/Repos/PageRepo.php b/app/Entities/Repos/PageRepo.php index f081b33dc..63e8b8370 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -18,6 +18,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; use BookStack\References\ReferenceStore; use BookStack\References\ReferenceUpdater; +use BookStack\Util\DatabaseTransaction; use Exception; class PageRepo @@ -61,8 +62,10 @@ class PageRepo ]); } - $page->save(); - $page->refresh()->rebuildPermissions(); + (new DatabaseTransaction(function () use ($page) { + $page->save(); + $page->refresh()->rebuildPermissions(); + }))->run(); return $page; } @@ -72,21 +75,23 @@ class PageRepo */ public function publishDraft(Page $draft, array $input): Page { - $draft->draft = false; - $draft->revision_count = 1; - $draft->priority = $this->getNewPriority($draft); - $this->updateTemplateStatusAndContentFromInput($draft, $input); - $this->baseRepo->update($draft, $input); - $draft->rebuildPermissions(); + return (new DatabaseTransaction(function () use ($draft, $input) { + $draft->draft = false; + $draft->revision_count = 1; + $draft->priority = $this->getNewPriority($draft); + $this->updateTemplateStatusAndContentFromInput($draft, $input); + $this->baseRepo->update($draft, $input); + $draft->rebuildPermissions(); - $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); - $this->revisionRepo->storeNewForPage($draft, $summary); - $draft->refresh(); + $summary = trim($input['summary'] ?? '') ?: trans('entities.pages_initial_revision'); + $this->revisionRepo->storeNewForPage($draft, $summary); + $draft->refresh(); - Activity::add(ActivityType::PAGE_CREATE, $draft); - $this->baseRepo->sortParent($draft); + Activity::add(ActivityType::PAGE_CREATE, $draft); + $this->baseRepo->sortParent($draft); - return $draft; + return $draft; + }))->run(); } /** @@ -117,7 +122,7 @@ class PageRepo $page->revision_count++; $page->save(); - // Remove all update drafts for this user & page. + // Remove all update drafts for this user and page. $this->revisionRepo->deleteDraftsForCurrentUser($page); // Save a revision after updating @@ -270,16 +275,18 @@ class PageRepo throw new PermissionsException('User does not have permission to create a page within the new parent'); } - $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : null; - $newBookId = ($parent instanceof Chapter) ? $parent->book->id : $parent->id; - $page->changeBook($newBookId); - $page->rebuildPermissions(); + return (new DatabaseTransaction(function () use ($page, $parent) { + $page->chapter_id = ($parent instanceof Chapter) ? $parent->id : null; + $newBookId = ($parent instanceof Chapter) ? $parent->book->id : $parent->id; + $page->changeBook($newBookId); + $page->rebuildPermissions(); - Activity::add(ActivityType::PAGE_MOVE, $page); + Activity::add(ActivityType::PAGE_MOVE, $page); - $this->baseRepo->sortParent($page); + $this->baseRepo->sortParent($page); - return $parent; + return $parent; + }))->run(); } /** diff --git a/app/Entities/Tools/HierarchyTransformer.php b/app/Entities/Tools/HierarchyTransformer.php index cd6c548fe..b0d8880f4 100644 --- a/app/Entities/Tools/HierarchyTransformer.php +++ b/app/Entities/Tools/HierarchyTransformer.php @@ -13,17 +13,12 @@ use BookStack\Facades\Activity; class HierarchyTransformer { - protected BookRepo $bookRepo; - protected BookshelfRepo $shelfRepo; - protected Cloner $cloner; - protected TrashCan $trashCan; - - public function __construct(BookRepo $bookRepo, BookshelfRepo $shelfRepo, Cloner $cloner, TrashCan $trashCan) - { - $this->bookRepo = $bookRepo; - $this->shelfRepo = $shelfRepo; - $this->cloner = $cloner; - $this->trashCan = $trashCan; + public function __construct( + protected BookRepo $bookRepo, + protected BookshelfRepo $shelfRepo, + protected Cloner $cloner, + protected TrashCan $trashCan + ) { } /** diff --git a/app/Entities/Tools/TrashCan.php b/app/Entities/Tools/TrashCan.php index 39c982cdc..5e8a93719 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -15,6 +15,7 @@ use BookStack\Exceptions\NotifyException; use BookStack\Facades\Activity; use BookStack\Uploads\AttachmentService; use BookStack\Uploads\ImageService; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Carbon; @@ -357,25 +358,26 @@ class TrashCan /** * Destroy the given entity. + * Returns the number of total entities destroyed in the operation. * * @throws Exception */ public function destroyEntity(Entity $entity): int { - if ($entity instanceof Page) { - return $this->destroyPage($entity); - } - if ($entity instanceof Chapter) { - return $this->destroyChapter($entity); - } - if ($entity instanceof Book) { - return $this->destroyBook($entity); - } - if ($entity instanceof Bookshelf) { - return $this->destroyShelf($entity); - } + $result = (new DatabaseTransaction(function () use ($entity) { + if ($entity instanceof Page) { + return $this->destroyPage($entity); + } else if ($entity instanceof Chapter) { + return $this->destroyChapter($entity); + } else if ($entity instanceof Book) { + return $this->destroyBook($entity); + } else if ($entity instanceof Bookshelf) { + return $this->destroyShelf($entity); + } + return null; + }))->run(); - return 0; + return $result ?? 0; } /** diff --git a/app/Permissions/JointPermissionBuilder.php b/app/Permissions/JointPermissionBuilder.php index c2922cdc9..56b22ad16 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -29,7 +29,7 @@ class JointPermissionBuilder /** * Re-generate all entity permission from scratch. */ - public function rebuildForAll() + public function rebuildForAll(): void { JointPermission::query()->truncate(); @@ -51,7 +51,7 @@ class JointPermissionBuilder /** * Rebuild the entity jointPermissions for a particular entity. */ - public function rebuildForEntity(Entity $entity) + public function rebuildForEntity(Entity $entity): void { $entities = [$entity]; if ($entity instanceof Book) { @@ -119,7 +119,7 @@ class JointPermissionBuilder /** * Build joint permissions for the given book and role combinations. */ - protected function buildJointPermissionsForBooks(EloquentCollection $books, array $roles, bool $deleteOld = false) + protected function buildJointPermissionsForBooks(EloquentCollection $books, array $roles, bool $deleteOld = false): void { $entities = clone $books; @@ -143,7 +143,7 @@ class JointPermissionBuilder /** * Rebuild the entity jointPermissions for a collection of entities. */ - protected function buildJointPermissionsForEntities(array $entities) + protected function buildJointPermissionsForEntities(array $entities): void { $roles = Role::query()->get()->values()->all(); $this->deleteManyJointPermissionsForEntities($entities); @@ -155,21 +155,19 @@ class JointPermissionBuilder * * @param Entity[] $entities */ - protected function deleteManyJointPermissionsForEntities(array $entities) + protected function deleteManyJointPermissionsForEntities(array $entities): void { $simpleEntities = $this->entitiesToSimpleEntities($entities); $idsByType = $this->entitiesToTypeIdMap($simpleEntities); - DB::transaction(function () use ($idsByType) { - foreach ($idsByType as $type => $ids) { - foreach (array_chunk($ids, 1000) as $idChunk) { - DB::table('joint_permissions') - ->where('entity_type', '=', $type) - ->whereIn('entity_id', $idChunk) - ->delete(); - } + foreach ($idsByType as $type => $ids) { + foreach (array_chunk($ids, 1000) as $idChunk) { + DB::table('joint_permissions') + ->where('entity_type', '=', $type) + ->whereIn('entity_id', $idChunk) + ->delete(); } - }); + } } /** @@ -195,7 +193,7 @@ class JointPermissionBuilder * @param Entity[] $originalEntities * @param Role[] $roles */ - protected function createManyJointPermissions(array $originalEntities, array $roles) + protected function createManyJointPermissions(array $originalEntities, array $roles): void { $entities = $this->entitiesToSimpleEntities($originalEntities); $jointPermissions = []; @@ -225,11 +223,9 @@ class JointPermissionBuilder } } - DB::transaction(function () use ($jointPermissions) { - foreach (array_chunk($jointPermissions, 1000) as $jointPermissionChunk) { - DB::table('joint_permissions')->insert($jointPermissionChunk); - } - }); + foreach (array_chunk($jointPermissions, 1000) as $jointPermissionChunk) { + DB::table('joint_permissions')->insert($jointPermissionChunk); + } } /** diff --git a/app/Permissions/PermissionsController.php b/app/Permissions/PermissionsController.php index 5d2035870..9dcfe242e 100644 --- a/app/Permissions/PermissionsController.php +++ b/app/Permissions/PermissionsController.php @@ -7,6 +7,7 @@ use BookStack\Entities\Tools\PermissionsUpdater; use BookStack\Http\Controller; use BookStack\Permissions\Models\EntityPermission; use BookStack\Users\Models\Role; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; class PermissionsController extends Controller @@ -40,7 +41,9 @@ class PermissionsController extends Controller $page = $this->queries->pages->findVisibleBySlugsOrFail($bookSlug, $pageSlug); $this->checkOwnablePermission('restrictions-manage', $page); - $this->permissionsUpdater->updateFromPermissionsForm($page, $request); + (new DatabaseTransaction(function () use ($page, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($page, $request); + }))->run(); $this->showSuccessNotification(trans('entities.pages_permissions_success')); @@ -70,7 +73,9 @@ class PermissionsController extends Controller $chapter = $this->queries->chapters->findVisibleBySlugsOrFail($bookSlug, $chapterSlug); $this->checkOwnablePermission('restrictions-manage', $chapter); - $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request); + (new DatabaseTransaction(function () use ($chapter, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($chapter, $request); + }))->run(); $this->showSuccessNotification(trans('entities.chapters_permissions_success')); @@ -100,7 +105,9 @@ class PermissionsController extends Controller $book = $this->queries->books->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $book); - $this->permissionsUpdater->updateFromPermissionsForm($book, $request); + (new DatabaseTransaction(function () use ($book, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($book, $request); + }))->run(); $this->showSuccessNotification(trans('entities.books_permissions_updated')); @@ -130,7 +137,9 @@ class PermissionsController extends Controller $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); - $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request); + (new DatabaseTransaction(function () use ($shelf, $request) { + $this->permissionsUpdater->updateFromPermissionsForm($shelf, $request); + }))->run(); $this->showSuccessNotification(trans('entities.shelves_permissions_updated')); @@ -145,7 +154,10 @@ class PermissionsController extends Controller $shelf = $this->queries->shelves->findVisibleBySlugOrFail($slug); $this->checkOwnablePermission('restrictions-manage', $shelf); - $updateCount = $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf); + $updateCount = (new DatabaseTransaction(function () use ($shelf) { + return $this->permissionsUpdater->updateBookPermissionsFromShelf($shelf); + }))->run(); + $this->showSuccessNotification(trans('entities.shelves_copy_permission_success', ['count' => $updateCount])); return redirect($shelf->getUrl()); diff --git a/app/Permissions/PermissionsRepo.php b/app/Permissions/PermissionsRepo.php index b41612968..6ced7b751 100644 --- a/app/Permissions/PermissionsRepo.php +++ b/app/Permissions/PermissionsRepo.php @@ -7,6 +7,7 @@ use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; use BookStack\Permissions\Models\RolePermission; use BookStack\Users\Models\Role; +use BookStack\Util\DatabaseTransaction; use Exception; use Illuminate\Database\Eloquent\Collection; @@ -48,38 +49,42 @@ class PermissionsRepo */ public function saveNewRole(array $roleData): Role { - $role = new Role($roleData); - $role->mfa_enforced = boolval($roleData['mfa_enforced'] ?? false); - $role->save(); + return (new DatabaseTransaction(function () use ($roleData) { + $role = new Role($roleData); + $role->mfa_enforced = boolval($roleData['mfa_enforced'] ?? false); + $role->save(); - $permissions = $roleData['permissions'] ?? []; - $this->assignRolePermissions($role, $permissions); - $this->permissionBuilder->rebuildForRole($role); + $permissions = $roleData['permissions'] ?? []; + $this->assignRolePermissions($role, $permissions); + $this->permissionBuilder->rebuildForRole($role); - Activity::add(ActivityType::ROLE_CREATE, $role); + Activity::add(ActivityType::ROLE_CREATE, $role); - return $role; + return $role; + }))->run(); } /** * Updates an existing role. - * Ensures Admin system role always have core permissions. + * Ensures the Admin system role always has core permissions. */ public function updateRole($roleId, array $roleData): Role { $role = $this->getRoleById($roleId); - if (isset($roleData['permissions'])) { - $this->assignRolePermissions($role, $roleData['permissions']); - } + return (new DatabaseTransaction(function () use ($role, $roleData) { + if (isset($roleData['permissions'])) { + $this->assignRolePermissions($role, $roleData['permissions']); + } - $role->fill($roleData); - $role->save(); - $this->permissionBuilder->rebuildForRole($role); + $role->fill($roleData); + $role->save(); + $this->permissionBuilder->rebuildForRole($role); - Activity::add(ActivityType::ROLE_UPDATE, $role); + Activity::add(ActivityType::ROLE_UPDATE, $role); - return $role; + return $role; + }))->run(); } /** @@ -114,7 +119,7 @@ class PermissionsRepo /** * Delete a role from the system. * Check it's not an admin role or set as default before deleting. - * If a migration Role ID is specified the users assign to the current role + * If a migration Role ID is specified, the users assigned to the current role * will be added to the role of the specified id. * * @throws PermissionsException @@ -131,17 +136,19 @@ class PermissionsRepo throw new PermissionsException(trans('errors.role_registration_default_cannot_delete')); } - if ($migrateRoleId !== 0) { - $newRole = Role::query()->find($migrateRoleId); - if ($newRole) { - $users = $role->users()->pluck('id')->toArray(); - $newRole->users()->sync($users); + (new DatabaseTransaction(function () use ($migrateRoleId, $role) { + if ($migrateRoleId !== 0) { + $newRole = Role::query()->find($migrateRoleId); + if ($newRole) { + $users = $role->users()->pluck('id')->toArray(); + $newRole->users()->sync($users); + } } - } - $role->entityPermissions()->delete(); - $role->jointPermissions()->delete(); - Activity::add(ActivityType::ROLE_DELETE, $role); - $role->delete(); + $role->entityPermissions()->delete(); + $role->jointPermissions()->delete(); + Activity::add(ActivityType::ROLE_DELETE, $role); + $role->delete(); + }))->run(); } } diff --git a/app/Sorting/BookSortController.php b/app/Sorting/BookSortController.php index 479d19724..d70d0e656 100644 --- a/app/Sorting/BookSortController.php +++ b/app/Sorting/BookSortController.php @@ -7,6 +7,7 @@ use BookStack\Entities\Queries\BookQueries; use BookStack\Entities\Tools\BookContents; use BookStack\Facades\Activity; use BookStack\Http\Controller; +use BookStack\Util\DatabaseTransaction; use Illuminate\Http\Request; class BookSortController extends Controller @@ -55,16 +56,18 @@ class BookSortController extends Controller // Sort via map if ($request->filled('sort-tree')) { - $sortMap = BookSortMap::fromJson($request->get('sort-tree')); - $booksInvolved = $sorter->sortUsingMap($sortMap); + (new DatabaseTransaction(function () use ($book, $request, $sorter, &$loggedActivityForBook) { + $sortMap = BookSortMap::fromJson($request->get('sort-tree')); + $booksInvolved = $sorter->sortUsingMap($sortMap); - // Rebuild permissions and add activity for involved books. - foreach ($booksInvolved as $bookInvolved) { - Activity::add(ActivityType::BOOK_SORT, $bookInvolved); - if ($bookInvolved->id === $book->id) { - $loggedActivityForBook = true; + // Add activity for involved books. + foreach ($booksInvolved as $bookInvolved) { + Activity::add(ActivityType::BOOK_SORT, $bookInvolved); + if ($bookInvolved->id === $book->id) { + $loggedActivityForBook = true; + } } - } + }))->run(); } if ($request->filled('auto-sort')) { diff --git a/app/Util/DatabaseTransaction.php b/app/Util/DatabaseTransaction.php new file mode 100644 index 000000000..e36bd2ef3 --- /dev/null +++ b/app/Util/DatabaseTransaction.php @@ -0,0 +1,42 @@ +callback); + } +} From 35a51197cee66385f61a0f8616aac3ff232dc089 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 6 Jul 2025 22:52:06 +0100 Subject: [PATCH 3/3] Perms: Fixed some issues made when adding transactions --- app/Entities/Repos/BookshelfRepo.php | 1 + app/Entities/Repos/ChapterRepo.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/app/Entities/Repos/BookshelfRepo.php b/app/Entities/Repos/BookshelfRepo.php index 5d4d604bc..8e60f58c4 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -30,6 +30,7 @@ class BookshelfRepo $this->baseRepo->updateCoverImage($shelf, $input['image'] ?? null); $this->updateBooks($shelf, $bookIds); Activity::add(ActivityType::BOOKSHELF_CREATE, $shelf); + return $shelf; }))->run(); } diff --git a/app/Entities/Repos/ChapterRepo.php b/app/Entities/Repos/ChapterRepo.php index 2ab6e671f..6503e63cf 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -37,6 +37,8 @@ class ChapterRepo Activity::add(ActivityType::CHAPTER_CREATE, $chapter); $this->baseRepo->sortParent($chapter); + + return $chapter; }))->run(); }