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 151d5b0555b..ac5a44e679d 100644 --- a/app/Entities/Repos/BaseRepo.php +++ b/app/Entities/Repos/BaseRepo.php @@ -77,7 +77,6 @@ public function update(Entity $entity, array $input) $entity->touch(); } - $entity->rebuildPermissions(); $entity->indexForSearch(); $this->referenceStore->updateForEntity($entity); @@ -139,7 +138,7 @@ public function updateDefaultTemplate(Book|Chapter $entity, int $templateId): vo /** * 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 c3be6d826a2..f081b33dc3a 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -77,6 +77,7 @@ public function publishDraft(Page $draft, array $input): Page $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 @@ public function publishDraft(Page $draft, array $input): Page /** * 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 6710f070ad6..cf41a6a94d0 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 | 33 +++++----- app/Entities/Repos/PageRepo.php | 57 +++++++++-------- 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, 223 insertions(+), 145 deletions(-) create mode 100644 app/Util/DatabaseTransaction.php diff --git a/app/Entities/Controllers/BookController.php b/app/Entities/Controllers/BookController.php index b1685081a3d..5d3d67f645c 100644 --- a/app/Entities/Controllers/BookController.php +++ b/app/Entities/Controllers/BookController.php @@ -18,6 +18,7 @@ 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 @@ public function convertToShelf(HierarchyTransformer $transformer, string $bookSl $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 4274589e260..677745500c3 100644 --- a/app/Entities/Controllers/ChapterController.php +++ b/app/Entities/Controllers/ChapterController.php @@ -18,6 +18,7 @@ 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 @@ public function convertToBook(HierarchyTransformer $transformer, string $bookSlu $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 92e6a81c337..6d28d5d6aab 100644 --- a/app/Entities/Repos/BookRepo.php +++ b/app/Entities/Repos/BookRepo.php @@ -10,6 +10,7 @@ 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 @@ public function __construct( */ 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 a00349ef1ae..5d4d604bc30 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -7,6 +7,7 @@ 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 @@ public function __construct( */ 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 fdf2de4e202..2ab6e671f6d 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -11,6 +11,7 @@ use BookStack\Exceptions\MoveOperationException; use BookStack\Exceptions\PermissionsException; use BookStack\Facades\Activity; +use BookStack\Util\DatabaseTransaction; use Exception; class ChapterRepo @@ -27,16 +28,16 @@ public function __construct( */ 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); - - $this->baseRepo->sortParent($chapter); - - return $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); + }))->run(); } /** @@ -88,12 +89,14 @@ public function move(Chapter $chapter, string $parentIdentifier): Book 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 f081b33dc3a..63e8b8370ee 100644 --- a/app/Entities/Repos/PageRepo.php +++ b/app/Entities/Repos/PageRepo.php @@ -18,6 +18,7 @@ use BookStack\Facades\Activity; use BookStack\References\ReferenceStore; use BookStack\References\ReferenceUpdater; +use BookStack\Util\DatabaseTransaction; use Exception; class PageRepo @@ -61,8 +62,10 @@ public function getNewDraftPage(Entity $parent) ]); } - $page->save(); - $page->refresh()->rebuildPermissions(); + (new DatabaseTransaction(function () use ($page) { + $page->save(); + $page->refresh()->rebuildPermissions(); + }))->run(); return $page; } @@ -72,21 +75,23 @@ public function getNewDraftPage(Entity $parent) */ 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(); - - $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); - - return $draft; + 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(); + + Activity::add(ActivityType::PAGE_CREATE, $draft); + $this->baseRepo->sortParent($draft); + + return $draft; + }))->run(); } /** @@ -117,7 +122,7 @@ public function update(Page $page, array $input): Page $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 @@ public function move(Page $page, string $parentIdentifier): Entity 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 cd6c548fe58..b0d8880f402 100644 --- a/app/Entities/Tools/HierarchyTransformer.php +++ b/app/Entities/Tools/HierarchyTransformer.php @@ -13,17 +13,12 @@ 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 39c982cdc92..5e8a9371942 100644 --- a/app/Entities/Tools/TrashCan.php +++ b/app/Entities/Tools/TrashCan.php @@ -15,6 +15,7 @@ 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 @@ protected function restoreEntity(Entity $entity): int /** * 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 c2922cdc961..56b22ad1604 100644 --- a/app/Permissions/JointPermissionBuilder.php +++ b/app/Permissions/JointPermissionBuilder.php @@ -29,7 +29,7 @@ public function __construct( /** * Re-generate all entity permission from scratch. */ - public function rebuildForAll() + public function rebuildForAll(): void { JointPermission::query()->truncate(); @@ -51,7 +51,7 @@ public function rebuildForAll() /** * 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 @@ protected function bookFetchQuery(): Builder /** * 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 @@ protected function buildJointPermissionsForBooks(EloquentCollection $books, arra /** * 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 @@ protected function buildJointPermissionsForEntities(array $entities) * * @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 @@ protected function entitiesToSimpleEntities(array $entities): array * @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 @@ protected function createManyJointPermissions(array $originalEntities, array $ro } } - 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 5d2035870bc..9dcfe242ec0 100644 --- a/app/Permissions/PermissionsController.php +++ b/app/Permissions/PermissionsController.php @@ -7,6 +7,7 @@ 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 @@ public function updateForPage(Request $request, string $bookSlug, string $pageSl $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 @@ public function updateForChapter(Request $request, string $bookSlug, string $cha $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 @@ public function updateForBook(Request $request, string $slug) $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 @@ public function updateForShelf(Request $request, string $slug) $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 @@ public function copyShelfPermissionsToBooks(string $slug) $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 b41612968b4..6ced7b7511c 100644 --- a/app/Permissions/PermissionsRepo.php +++ b/app/Permissions/PermissionsRepo.php @@ -7,6 +7,7 @@ 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 @@ public function getRoleById(int $id): Role */ 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 @@ protected function assignRolePermissions(Role $role, array $permissionNameArray /** * 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 @@ public function deleteRole(int $roleId, int $migrateRoleId = 0): void 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 479d1972440..d70d0e6565a 100644 --- a/app/Sorting/BookSortController.php +++ b/app/Sorting/BookSortController.php @@ -7,6 +7,7 @@ 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 @@ public function update(Request $request, BookSorter $sorter, string $bookSlug) // 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 00000000000..e36bd2ef310 --- /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 5d4d604bc30..8e60f58c42f 100644 --- a/app/Entities/Repos/BookshelfRepo.php +++ b/app/Entities/Repos/BookshelfRepo.php @@ -30,6 +30,7 @@ public function create(array $input, array $bookIds): Bookshelf $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 2ab6e671f6d..6503e63cfaf 100644 --- a/app/Entities/Repos/ChapterRepo.php +++ b/app/Entities/Repos/ChapterRepo.php @@ -37,6 +37,8 @@ public function create(array $input, Book $parentBook): Chapter Activity::add(ActivityType::CHAPTER_CREATE, $chapter); $this->baseRepo->sortParent($chapter); + + return $chapter; }))->run(); }