Skip to content

Commit 4e406c4

Browse files
ssddanbrownssddanbrown
committed
Attachments: Aligned update form permission checks with other actions
Thanks to Ashutosh Jena(MAVERICK-VF142) for reporting. Not considered a significant security issue since it already required page update permissions, which would generally be considered higher privileged than the added page view.
1 parent 6107161 commit 4e406c4

2 files changed

Lines changed: 22 additions & 1 deletion

File tree

‎app/Uploads/Controllers/AttachmentController.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,9 @@ public function getUpdateForm(string $attachmentId)
9393
/** @var Attachment $attachment */
9494
$attachment = Attachment::query()->findOrFail($attachmentId);
9595

96+
$this->checkOwnablePermission(Permission::PageView, $attachment->page);
9697
$this->checkOwnablePermission(Permission::PageUpdate, $attachment->page);
97-
$this->checkOwnablePermission(Permission::AttachmentCreate, $attachment);
98+
$this->checkOwnablePermission(Permission::AttachmentUpdate, $attachment);
9899

99100
return view('attachments.manager-edit-form', [
100101
'attachment' => $attachment,

‎tests/Uploads/AttachmentTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,26 @@ public function test_attachment_access_without_permission_shows_404()
274274
$this->files->deleteAllAttachmentFiles();
275275
}
276276

277+
public function test_attachment_edit_form_access_requires_view_permission()
278+
{
279+
$page = $this->entities->page();
280+
/** @var Attachment $attachment */
281+
$attachment = Attachment::factory()->create(['uploaded_to' => $page->id]);
282+
$editor = $this->users->editor();
283+
284+
$this->permissions->disableEntityInheritedPermissions($page);
285+
$this->permissions->grantUserRolePermissions($editor, [Permission::AttachmentUpdateAll]);
286+
$this->permissions->setEntityPermissionsForRole($page, ['update'], $editor->roles()->first());
287+
288+
$resp = $this->actingAs($editor)->get("/attachments/edit/{$attachment->id}");
289+
$this->assertPermissionError($resp);
290+
291+
$this->permissions->setEntityPermissionsForRole($page, ['view', 'update'], $editor->roles()->first());
292+
$resp = $this->actingAs($editor)->get("/attachments/edit/{$attachment->id}");
293+
$resp->assertOk();
294+
$resp->assertSee($attachment->name);
295+
}
296+
277297
public function test_data_and_js_links_cannot_be_attached_to_a_page()
278298
{
279299
$page = $this->entities->page();

0 commit comments

Comments
 (0)