Skip to content

Commit 2d6e60a

Browse files
ssddanbrownssddanbrown
committed
Merge branch 'sec_aug26' into development
2 parents df5f774 + 0365dcc commit 2d6e60a

32 files changed

Lines changed: 377 additions & 53 deletions

‎app/Entities/Controllers/PageController.php‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use BookStack\Activity\Tools\UserEntityWatchOptions;
88
use BookStack\Entities\Models\Book;
99
use BookStack\Entities\Models\Chapter;
10+
use BookStack\Entities\Models\Page;
1011
use BookStack\Entities\Queries\EntityQueries;
1112
use BookStack\Entities\Queries\PageQueries;
1213
use BookStack\Entities\Repos\PageRepo;
@@ -95,14 +96,15 @@ public function createAsGuest(Request $request, string $bookSlug, ?string $chapt
9596
}
9697

9798
/**
98-
* Show form to continue editing a draft page.
99+
* Show a form to continue editing a draft page.
99100
*
100101
* @throws NotFoundException
101102
*/
102103
public function editDraft(Request $request, string $bookSlug, int $pageId)
103104
{
104105
$draft = $this->queries->findVisibleByIdOrFail($pageId);
105106
$this->checkOwnablePermission(Permission::PageCreate, $draft->getParent());
107+
$this->ensureDraftAccess($draft);
106108

107109
$editorData = new PageEditorData($draft, $this->entityQueries, $request->query('editor', ''));
108110
$this->setPageTitle(trans('entities.pages_edit_draft'));
@@ -124,6 +126,7 @@ public function store(Request $request, string $bookSlug, int $pageId)
124126

125127
$draftPage = $this->queries->findVisibleByIdOrFail($pageId);
126128
$this->checkOwnablePermission(Permission::PageCreate, $draftPage->getParent());
129+
$this->ensureDraftAccess($draftPage);
127130

128131
$page = $this->pageRepo->publishDraft($draftPage, $request->all());
129132

@@ -235,6 +238,7 @@ public function update(Request $request, string $bookSlug, string $pageSlug)
235238
* Save a draft update as a revision.
236239
*
237240
* @throws NotFoundException
241+
* @throws PermissionsException
238242
*/
239243
public function saveDraft(Request $request, int $pageId)
240244
{
@@ -245,6 +249,10 @@ public function saveDraft(Request $request, int $pageId)
245249
return $this->jsonError(trans('errors.guests_cannot_save_drafts'), 500);
246250
}
247251

252+
if ($page->draft) {
253+
$this->ensureDraftAccess($page);
254+
}
255+
248256
$draft = $this->pageRepo->updatePageDraft($page, $request->only(['name', 'html', 'markdown']));
249257
$warnings = (new PageEditActivity($page))->getWarningMessagesForDraft($draft);
250258

@@ -294,11 +302,14 @@ public function showDelete(string $bookSlug, string $pageSlug)
294302
* Show the deletion page for the specified page.
295303
*
296304
* @throws NotFoundException
305+
* @throws PermissionsException
297306
*/
298307
public function showDeleteDraft(string $bookSlug, int $pageId)
299308
{
300309
$page = $this->queries->findVisibleByIdOrFail($pageId);
301310
$this->checkOwnablePermission(Permission::PageUpdate, $page);
311+
$this->ensureDraftAccess($page);
312+
302313
$this->setPageTitle(trans('entities.pages_delete_draft_named', ['pageName' => $page->getShortName()]));
303314
$usedAsTemplate =
304315
$this->entityQueries->books->start()->where('default_template_id', '=', $page->id)->count() > 0 ||
@@ -340,7 +351,9 @@ public function destroyDraft(string $bookSlug, int $pageId)
340351
$page = $this->queries->findVisibleByIdOrFail($pageId);
341352
$book = $page->book;
342353
$chapter = $page->chapter;
354+
343355
$this->checkOwnablePermission(Permission::PageUpdate, $page);
356+
$this->ensureDraftAccess($page);
344357

345358
$this->pageRepo->destroy($page);
346359

@@ -470,4 +483,14 @@ public function copy(Request $request, Cloner $cloner, string $bookSlug, string
470483

471484
return redirect($pageCopy->getUrl());
472485
}
486+
487+
/**
488+
* @throws PermissionsException
489+
*/
490+
protected function ensureDraftAccess(Page $draft): void
491+
{
492+
if (!$draft->draft || $draft->created_by !== user()->id) {
493+
throw new PermissionsException('This page is already published or does not belong to you.');
494+
}
495+
}
473496
}

‎app/Entities/Models/EntityTable.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ class EntityTable extends Model
2525

2626
/**
2727
* Get the entities that are visible to the current user.
28+
* Note: This only applies basic permission filtering which considers the core entity table.
29+
* This will not filter on information from other tables such as page draft status.
30+
* That should be done after applying this scope.
2831
*/
2932
public function scopeVisible(Builder $query): Builder
3033
{

‎app/Entities/Queries/EntityQueries.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,13 @@ public function visibleForList(): Builder
8181
})->leftJoin('entity_page_data', function (JoinClause $join) {
8282
$join->on('entity_page_data.page_id', '=', 'entities.id')
8383
->where('entities.type', '=', 'page');
84+
})->where(function ($query) {
85+
$query->whereNull('entity_page_data.draft')
86+
->orWhere('entity_page_data.draft', '=', 0)
87+
->orWhere(function ($query) {
88+
$query->where('entity_page_data.draft', '=', 1)
89+
->where('entities.owned_by', '=', user()->id);
90+
});
8491
});
8592
}
8693

‎app/Entities/Tools/TrashCan.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,8 @@ protected function restoreEntity(Entity $entity): int
358358
$entity->chapters()->withTrashed()->withCount('deletions')->get()->each($restoreAction);
359359
}
360360

361+
$entity->rebuildPermissions();
362+
361363
return $count;
362364
}
363365

‎app/Exceptions/Handler.php‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,4 @@ protected function unauthenticated($request, AuthenticationException $exception)
169169

170170
return redirect()->guest('login');
171171
}
172-
173-
/**
174-
* Convert a validation exception into a JSON response.
175-
*
176-
* @param Request $request
177-
*/
178-
protected function invalidJson($request, ValidationException $exception): JsonResponse
179-
{
180-
return response()->json($exception->errors(), $exception->status);
181-
}
182172
}

‎app/Exports/ZipExports/Models/ZipExportBook.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public static function validate(ZipValidationHelper $context, array $data): arra
8787
'id' => ['nullable', 'int', $context->uniqueIdRule('book')],
8888
'name' => ['required', 'string', 'min:1'],
8989
'description_html' => ['nullable', 'string'],
90-
'cover' => ['nullable', 'string', $context->fileReferenceRule()],
90+
'cover' => ['nullable', 'string', $context->imageFileReferenceRule()],
9191
'tags' => ['array'],
9292
'pages' => ['array'],
9393
'chapters' => ['array'],

‎app/Exports/ZipExports/Models/ZipExportImage.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@ public function metadataOnly(): void
3232

3333
public static function validate(ZipValidationHelper $context, array $data): array
3434
{
35-
$acceptedImageTypes = ['image/png', 'image/jpeg', 'image/gif', 'image/webp'];
3635
$rules = [
3736
'id' => ['nullable', 'int', $context->uniqueIdRule('image')],
3837
'name' => ['required', 'string', 'min:1'],
39-
'file' => ['required', 'string', $context->fileReferenceRule($acceptedImageTypes)],
38+
'file' => ['required', 'string', $context->imageFileReferenceRule()],
4039
'type' => ['required', 'string', Rule::in(['gallery', 'drawio'])],
4140
];
4241

‎app/Exports/ZipExports/ZipExportFiles.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ protected function getAllFileNames(): array
8080
}
8181

8282
/**
83-
* Extract each of the ZIP export tracked files.
83+
* Extract each of the ZIP export-tracked files.
8484
* Calls the given callback for each tracked file, passing a temporary
8585
* file reference of the file contents, and the zip-local tracked reference.
8686
*/

‎app/Exports/ZipExports/ZipImportRunner.php‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public function run(Import $import, ?Entity $parent = null): Entity
9696

9797
/**
9898
* Revert any files which have been stored during this import process.
99-
* Considers files only, and avoids the database under the
99+
* Considers files only and avoids the database under the
100100
* assumption that the database may already have been
101101
* reverted as part of a transaction rollback.
102102
*/
@@ -129,7 +129,7 @@ protected function importBook(ZipExportBook $exportBook, ZipExportReader $reader
129129
$book = $this->bookRepo->create([
130130
'name' => $exportBook->name,
131131
'description_html' => $exportBook->description_html ?? '',
132-
'image' => $exportBook->cover ? $this->zipFileToUploadedFile($exportBook->cover, $reader) : null,
132+
'image' => $exportBook->cover ? $this->zipFileToUploadedFile($exportBook->cover, $reader, true) : null,
133133
'tags' => $this->exportTagsToInputArray($exportBook->tags),
134134
]);
135135

@@ -227,18 +227,11 @@ protected function importAttachment(ZipExportAttachment $exportAttachment, Page
227227

228228
protected function importImage(ZipExportImage $exportImage, Page $page, ZipExportReader $reader): Image
229229
{
230-
$mime = $reader->sniffFileMime($exportImage->file);
231-
$extension = explode('/', $mime)[1];
232-
233-
$file = $this->zipFileToUploadedFile($exportImage->file, $reader);
230+
$file = $this->zipFileToUploadedFile($exportImage->file, $reader, true);
234231
$image = $this->imageService->saveNewFromUpload(
235232
$file,
236233
$exportImage->type,
237234
$page->id,
238-
null,
239-
null,
240-
true,
241-
$exportImage->name . '.' . $extension,
242235
);
243236

244237
$image->name = $exportImage->name;
@@ -261,7 +254,7 @@ protected function exportTagsToInputArray(array $exportTags): array
261254
return $tags;
262255
}
263256

264-
protected function zipFileToUploadedFile(string $fileName, ZipExportReader $reader): UploadedFile
257+
protected function zipFileToUploadedFile(string $fileName, ZipExportReader $reader, bool $forceExtensionFromMime = false): UploadedFile
265258
{
266259
if (!$reader->fileWithinSizeLimit($fileName)) {
267260
throw new ZipImportException([
@@ -277,7 +270,16 @@ protected function zipFileToUploadedFile(string $fileName, ZipExportReader $read
277270

278271
$this->tempFilesToCleanup[] = $tempPath;
279272

280-
return new UploadedFile($tempPath, $fileName);
273+
$intendedUploadName = $fileName;
274+
if ($forceExtensionFromMime) {
275+
$mime = $reader->sniffFileMime($fileName);
276+
$extension = explode('/', $mime)[1];
277+
if (!str_ends_with(strtolower($intendedUploadName), '.' . $extension)) {
278+
$intendedUploadName .= '.' . $extension;
279+
}
280+
}
281+
282+
return new UploadedFile($tempPath, $intendedUploadName);
281283
}
282284

283285
/**

‎app/Exports/ZipExports/ZipValidationHelper.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace BookStack\Exports\ZipExports;
44

55
use BookStack\Exports\ZipExports\Models\ZipExportModel;
6+
use BookStack\Uploads\ImageService;
67
use Illuminate\Validation\Factory;
78

89
class ZipValidationHelper
@@ -38,6 +39,11 @@ public function fileReferenceRule(array $acceptedMimes = []): ZipFileReferenceRu
3839
return new ZipFileReferenceRule($this, $acceptedMimes);
3940
}
4041

42+
public function imageFileReferenceRule(): ZipFileReferenceRule
43+
{
44+
return new ZipFileReferenceRule($this, ImageService::getSupportedMimeTypes());
45+
}
46+
4147
public function uniqueIdRule(string $type): ZipUniqueIdRule
4248
{
4349
return new ZipUniqueIdRule($this, $type);

0 commit comments

Comments
 (0)