Images: Increased validation against related page on upload

Adds validation that the related page exists, is visible, and that the
user has page edit permissions for the page.
Aligns with attachment permission model, and aligns across different
image endpoints.

Added tests to cover.
Closes #6126
This commit is contained in:
Dan Brown
2026-05-14 18:10:46 +01:00
parent c5c066c3e7
commit c87abbd23f
14 changed files with 155 additions and 53 deletions

View File

@@ -117,7 +117,7 @@ class CommentMentionTest extends TestCase
$page = $this->entities->page();
$this->permissions->disableEntityInheritedPermissions($page);
$this->permissions->addEntityPermission($page, ['view'], $userA->roles()->first());
$this->permissions->setEntityPermissionsForRole($page, ['view'], $userA->roles()->first());
$this->asAdmin()->post("/comment/{$page->id}", [
'html' => '<p>Hello <a data-mention-user-id="' . $userA->id . '"></a> and <a data-mention-user-id="' . $userB->id . '"></a></p>'

View File

@@ -39,7 +39,7 @@ class ContentPermissionsApiTest extends TestCase
$page = $this->entities->page();
$owner = $this->users->newUser();
$role = $this->users->createRole();
$this->permissions->addEntityPermission($page, ['view', 'delete'], $role);
$this->permissions->setEntityPermissionsForRole($page, ['view', 'delete'], $role);
$this->permissions->changeEntityOwner($page, $owner);
$this->permissions->setFallbackPermissions($page, ['update', 'create']);
@@ -209,7 +209,7 @@ class ContentPermissionsApiTest extends TestCase
public function test_update_can_clear_roles_permissions()
{
$page = $this->entities->page();
$this->permissions->addEntityPermission($page, ['view'], $this->users->createRole());
$this->permissions->setEntityPermissionsForRole($page, ['view'], $this->users->createRole());
$page->owned_by = null;
$page->save();

View File

@@ -154,6 +154,34 @@ class ImageGalleryApiTest extends TestCase
$resp->assertStatus(404);
}
public function test_create_requires_update_permission_for_the_target_page()
{
$editor = $this->users->editor();
$this->actingAsForApi($editor);
$makeRequest = function (int $uploadedTo) {
return $this->call('POST', $this->baseEndpoint, [
'type' => 'gallery',
'uploaded_to' => $uploadedTo,
'name' => 'My awesome image!',
], [], [
'image' => $this->files->uploadedImage('my-cool-image.png'),
]);
};
$page = $this->entities->page();
$this->permissions->disableEntityInheritedPermissions($page);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $editor->roles()->first());
$resp = $makeRequest($page->id);
$resp->assertStatus(403);
$this->permissions->setEntityPermissionsForRole($page, ['view', 'update'], $editor->roles()->first());
$resp = $makeRequest($page->id);
$resp->assertStatus(200);
}
public function test_create_has_restricted_types()
{
$this->actingAsApiEditor();

View File

@@ -16,7 +16,7 @@ class RegeneratePermissionsCommandTest extends TestCase
$page = $this->entities->page();
$editor = $this->users->editor();
$role = $editor->roles()->first();
$this->permissions->addEntityPermission($page, ['view'], $role);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $role);
JointPermission::query()->truncate();
$this->assertDatabaseMissing('joint_permissions', ['entity_id' => $page->id]);

View File

@@ -2,7 +2,6 @@
namespace Tests;
use Illuminate\Foundation\Http\Middleware\ValidatePostSize;
use Illuminate\Support\Facades\Log;
class ErrorTest extends TestCase
@@ -44,7 +43,7 @@ class ErrorTest extends TestCase
$this->actingAs($editor)->get($page->getUrl())->assertOk();
$this->permissions->disableEntityInheritedPermissions($book);
$this->permissions->addEntityPermission($page, ['view'], $editor->roles()->first());
$this->permissions->setEntityPermissionsForRole($page, ['view'], $editor->roles()->first());
$resp = $this->actingAs($editor)->get($book->getUrl());
$resp->assertNotFound();

View File

@@ -101,8 +101,9 @@ class PermissionsProvider
$this->addEntityPermissionEntries($entity, $permissions);
}
public function addEntityPermission(Entity $entity, array $actionList, Role $role)
public function setEntityPermissionsForRole(Entity $entity, array $actionList, Role $role)
{
$entity->permissions()->where('role_id', '=', $role->id)->delete();
$permissionData = $this->actionListToEntityPermissionData($actionList, $role->id);
$this->addEntityPermissionEntries($entity, [$permissionData]);
}

View File

@@ -29,8 +29,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->disableEntityInheritedPermissions($page);
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->addEntityPermission($page, ['view'], $roleB);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleB);
$this->assertVisibleToUser($page, $user);
}
@@ -42,7 +42,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -54,7 +54,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -67,8 +67,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->addEntityPermission($chapter, ['view'], $roleB);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleB);
$this->assertVisibleToUser($page, $user);
}
@@ -80,8 +80,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->addEntityPermission($page, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -93,8 +93,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -107,8 +107,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->addEntityPermission($chapter, ['view'], $roleB);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleB);
$this->assertVisibleToUser($page, $user);
}
@@ -121,8 +121,8 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->disableEntityInheritedPermissions($chapter);
$this->permissions->addEntityPermission($page, ['view'], $roleA);
$this->permissions->addEntityPermission($chapter, [], $roleB);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleB);
$this->assertVisibleToUser($page, $user);
}
@@ -131,7 +131,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
{
[$user, $roleA] = $this->users->newUserWithRole();
$page = $this->entities->page();
$this->permissions->addEntityPermission($page, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -140,7 +140,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
{
[$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
$page = $this->entities->page();
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -150,7 +150,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
[$user, $roleA] = $this->users->newUserWithRole([], []);
$page = $this->entities->pageWithinChapter();
$chapter = $page->chapter;
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -160,7 +160,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
[$user, $roleA] = $this->users->newUserWithRole([], ['page-view-all']);
$page = $this->entities->pageWithinChapter();
$chapter = $page->chapter;
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -170,7 +170,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
[$user, $roleA] = $this->users->newUserWithRole([], ['page-view-own']);
$page = $this->entities->pageWithinChapter();
$chapter = $page->chapter;
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->permissions->changeEntityOwner($page, $user);
$this->assertNotVisibleToUser($page, $user);
@@ -182,7 +182,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$roleB = $this->users->attachNewRole($user);
$page = $this->entities->page();
$this->permissions->addEntityPermission($page, [], $roleB);
$this->permissions->setEntityPermissionsForRole($page, [], $roleB);
$this->assertNotVisibleToUser($page, $user);
}
@@ -194,7 +194,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->changeEntityOwner($page, $user);
$this->permissions->addEntityPermission($page, [], $roleB);
$this->permissions->setEntityPermissionsForRole($page, [], $roleB);
$this->assertNotVisibleToUser($page, $user);
}
@@ -207,7 +207,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->pageWithinChapter();
$chapter = $page->chapter;
$this->permissions->addEntityPermission($chapter, [], $roleB);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleB);
$this->assertNotVisibleToUser($page, $user);
}
@@ -220,7 +220,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->changeEntityOwner($page, $user);
$this->permissions->addEntityPermission($chapter, [], $roleB);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleB);
$this->assertNotVisibleToUser($page, $user);
}
@@ -231,7 +231,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->setFallbackPermissions($page, []);
$this->permissions->addEntityPermission($page, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -241,7 +241,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->setFallbackPermissions($page, ['view']);
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -253,7 +253,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->setFallbackPermissions($page, []);
$this->permissions->addEntityPermission($page, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -265,7 +265,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$page = $this->entities->page();
$this->permissions->setFallbackPermissions($page, ['view']);
$this->permissions->addEntityPermission($page, [], $roleA);
$this->permissions->setEntityPermissionsForRole($page, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -277,7 +277,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->setFallbackPermissions($chapter, []);
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -289,7 +289,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->setFallbackPermissions($chapter, ['view']);
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -302,7 +302,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->setFallbackPermissions($chapter, []);
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->assertVisibleToUser($page, $user);
}
@@ -315,7 +315,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$chapter = $page->chapter;
$this->permissions->setFallbackPermissions($chapter, ['view']);
$this->permissions->addEntityPermission($chapter, [], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, [], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -328,7 +328,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$this->permissions->setFallbackPermissions($chapter, []);
$this->permissions->setFallbackPermissions($page, []);
$this->permissions->addEntityPermission($chapter, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($chapter, ['view'], $roleA);
$this->assertNotVisibleToUser($page, $user);
}
@@ -342,7 +342,7 @@ class EntityRolePermissionsTest extends PermissionScenarioTestCase
$this->permissions->setFallbackPermissions($book, []);
$this->permissions->setFallbackPermissions($chapter, []);
$this->permissions->addEntityPermission($book, ['view'], $roleA);
$this->permissions->setEntityPermissionsForRole($book, ['view'], $roleA);
$this->assertNotVisibleToUser($page, $user);
}

View File

@@ -6,7 +6,6 @@ use BookStack\Entities\Models\Book;
use BookStack\Entities\Models\Chapter;
use BookStack\Permissions\Models\RolePermission;
use BookStack\Users\Models\Role;
use BookStack\Users\Models\User;
use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Facades\View;
@@ -173,7 +172,7 @@ class PublicActionTest extends TestCase
$newRole = $this->users->attachNewRole($this->users->guest(), []);
$page = $this->entities->page();
$this->permissions->disableEntityInheritedPermissions($page);
$this->permissions->addEntityPermission($page, ['view', 'update'], $newRole);
$this->permissions->setEntityPermissionsForRole($page, ['view', 'update'], $newRole);
$resp = $this->get($page->getUrl());
$resp->assertOk();

View File

@@ -72,6 +72,30 @@ class DrawioTest extends TestCase
$this->assertTrue($testImageData === $uploadedImageData, 'Uploaded image file data does not match our test image as expected');
}
public function test_base64_upload_requires_edit_permission_to_page()
{
$page = $this->entities->page();
$editor = $this->users->editor();
$this->actingAs($editor);
$this->permissions->disableEntityInheritedPermissions($page);
$upload = function () use ($page) {
return $this->postJson('images/drawio', [
'uploaded_to' => $page->id,
'image' => 'image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAUAAAAFCAIAAAACDbGyAAAACXBIWXMAAAsTAAALEwEAmpwYAAAAB3RJTUUH4gEcDCo5iYNs+gAAAB1pVFh0Q29tbWVudAAAAAAAQ3JlYXRlZCB3aXRoIEdJTVBkLmUHAAAAFElEQVQI12O0jN/KgASYGFABqXwAZtoBV6Sl3hIAAAAASUVORK5CYII=',
]);
};
$upload()->assertStatus(404);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $editor->roles()->first());
$upload()->assertStatus(403);
$this->permissions->setEntityPermissionsForRole($page, ['view', 'update'], $editor->roles()->first());
$upload()->assertStatus(200);
}
public function test_drawio_url_can_be_configured()
{
config()->set('services.drawio', 'http://cats.com?dog=tree');

View File

@@ -36,6 +36,45 @@ class ImageTest extends TestCase
]);
}
public function test_image_upload_with_page_reference_requires_visibility_and_update_permissions_of_target_page()
{
$page = $this->entities->page();
$editor = $this->users->editor();
$this->permissions->disableEntityInheritedPermissions($page);
$this->actingAs($editor);
$resp = $this->files->uploadGalleryImage($this, 'test-image.png', $page->id);
$resp->assertStatus(404);
$this->permissions->setEntityPermissionsForRole($page, ['view'], $editor->roles()->first());
$resp = $this->files->uploadGalleryImage($this, 'test-image.png', $page->id);
$this->assertPermissionError($resp);
$this->permissions->setEntityPermissionsForRole($page, ['view', 'update'], $editor->roles()->first());
$resp = $this->files->uploadGalleryImage($this, 'test-image.png', $page->id);
$resp->assertStatus(200);
$this->files->deleteAtRelativePath($resp->json('path'));
}
public function test_image_upload_with_page_reference_requires_page_to_exist()
{
$page = $this->entities->page();
$this->entities->destroy($page);
$editor = $this->users->editor();
$this->actingAs($editor);
$resp = $this->files->uploadGalleryImage($this, 'test-image.png', $page->id);
$resp->assertStatus(404);
$resp = $this->files->uploadGalleryImage($this, 'test-image.png', 0);
$resp->assertStatus(404);
}
public function test_image_display_thumbnail_generation_does_not_increase_image_size()
{
$page = $this->entities->page();