BookStackApp/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2026-10-12 02:01:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Attachment API: Fixed missing check on attachment page change

Browse Source 
On the change of an attachment uploaded_to (page) this ensures that a
PageUpdate permission check is made against the original attachment
page, in addition to the new page.

Thanks to Wade Sparks / ByteMe.Red for reporting.
This commit is contained in:
Dan Brown
2026-08-19 12:17:58 +01:00
parent c813c1b362
commit a889c18c02
2 changed files with 36 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/Uploads/Controllers/AttachmentApiController.php
+10 -7
View File
@@ -24,7 +24,7 @@ class AttachmentApiController extends ApiController
/**
* Get a listing of attachments visible to the user.
* The external property indicates whether the attachment is simple a link.
* The external property indicates whether the attachment is simply a link.
* A false value for the external property would indicate a file upload.
*/
public function list()
@@ -39,7 +39,7 @@ class AttachmentApiController extends ApiController
* An uploaded_to value must be provided containing an ID of the page
* that this upload will be related to.
*
* If you're uploading a file the POST data should be provided via
* If you're uploading a file, the POST data should be provided via
* a multipart/form-data type request instead of JSON.
*
* @throws ValidationException
@@ -71,7 +71,7 @@ class AttachmentApiController extends ApiController
}
/**
* Get the details & content of a single attachment of the given ID.
* Get the details and content of a single attachment of the given ID.
* The attachment link or file content is provided via a 'content' property.
* For files the content will be base64 encoded.
*
@@ -120,7 +120,7 @@ class AttachmentApiController extends ApiController
/**
* Update the details of a single attachment.
* As per the create endpoint, if a file is being provided as the attachment content
* As per the create endpoint, if a file is being provided as the attachment content,
* the request should be formatted as a multipart/form-data request instead of JSON.
*
* @throws ValidationException
@@ -134,9 +134,12 @@ class AttachmentApiController extends ApiController
$page = $attachment->page;
if ($requestData['uploaded_to'] ?? false) {
$pageId = $request->input('uploaded_to');
$page = $this->pageQueries->findVisibleByIdOrFail($pageId);
$attachment->uploaded_to = $requestData['uploaded_to'];
$pageId = intval($requestData['uploaded_to']);
if ($pageId !== $page->id) {
$this->checkOwnablePermission(Permission::PageUpdate, $page);
$page = $this->pageQueries->findVisibleByIdOrFail($pageId);
$attachment->uploaded_to = $pageId;
}
}
$this->checkOwnablePermission(Permission::PageView, $page);
tests/Api/AttachmentsApiTest.php
+26
View File
@@ -3,6 +3,7 @@
namespace Tests\Api;
use BookStack\Entities\Models\Page;
use BookStack\Permissions\Permission;
use BookStack\Uploads\Attachment;
use Illuminate\Http\UploadedFile;
use Illuminate\Testing\AssertableJsonString;
@@ -345,6 +346,31 @@ class AttachmentsApiTest extends TestCase
$this->assertEquals('', $attachment->extension);
}
public function test_update_uploaded_to_change_requires_edit_permission_to_old_page()
{
$editor = $this->users->editor();
$this->actingAsForApi($editor);
$originalPage = $this->entities->page();
$attachment = $this->createAttachmentForPage($originalPage);
$newPage = $this->entities->page();
$details = [
'name' => 'My updated API attachment',
'uploaded_to' => $newPage->id,
];
$this->permissions->setEntityPermissions($originalPage, ['view'], [$editor->roles->first()]);
$this->permissions->grantUserRolePermissions($editor, [Permission::AttachmentUpdateAll]);
$resp = $this->putJson("{$this->baseEndpoint}/{$attachment->id}", $details);
$attachment->refresh();
$this->assertPermissionError($resp);
$this->assertNotEquals($newPage->id, $attachment->uploaded_to);
$this->assertEquals($originalPage->id, $attachment->uploaded_to);
}
public function test_delete_endpoint()
{
$this->actingAsApiAdmin();