diff --git a/app/Http/Controllers/Api/AttachmentApiController.php b/app/Http/Controllers/Api/AttachmentApiController.php
index 2ee1c98a6..7aa4ee493 100644
--- a/app/Http/Controllers/Api/AttachmentApiController.php
+++ b/app/Http/Controllers/Api/AttachmentApiController.php
@@ -25,8 +25,8 @@ class AttachmentApiController extends ApiController
'update' => [
'name' => 'min:1|max:255|string',
'uploaded_to' => 'integer|exists:pages,id',
- 'file' => 'link|file',
- 'link' => 'file|min:1|max:255|safe_url'
+ 'file' => 'file',
+ 'link' => 'min:1|max:255|safe_url'
],
];
@@ -87,7 +87,9 @@ class AttachmentApiController extends ApiController
public function read(string $id)
{
/** @var Attachment $attachment */
- $attachment = Attachment::visible()->findOrFail($id);
+ $attachment = Attachment::visible()
+ ->with(['createdBy', 'updatedBy'])
+ ->findOrFail($id);
$attachment->setAttribute('links', [
'html' => $attachment->htmlLink(),
@@ -129,7 +131,7 @@ class AttachmentApiController extends ApiController
if ($request->hasFile('file')) {
$uploadedFile = $request->file('file');
- $attachment = $this->attachmentService->saveUpdatedUpload($uploadedFile, $page->id);
+ $attachment = $this->attachmentService->saveUpdatedUpload($uploadedFile, $attachment);
}
$this->attachmentService->updateFile($attachment, $requestData);
diff --git a/app/Uploads/Attachment.php b/app/Uploads/Attachment.php
index dfd7d980a..8ae53199e 100644
--- a/app/Uploads/Attachment.php
+++ b/app/Uploads/Attachment.php
@@ -3,6 +3,7 @@
namespace BookStack\Uploads;
use BookStack\Auth\Permissions\PermissionService;
+use BookStack\Auth\User;
use BookStack\Entities\Models\Entity;
use BookStack\Entities\Models\Page;
use BookStack\Model;
@@ -18,6 +19,8 @@ use Illuminate\Database\Eloquent\Relations\BelongsTo;
* @property ?Page $page
* @property bool $external
* @property int $uploaded_to
+ * @property User $updatedBy
+ * @property User $createdBy
*
* @method static Entity|Builder visible()
*/
@@ -26,6 +29,10 @@ class Attachment extends Model
use HasCreatorAndUpdater;
protected $fillable = ['name', 'order'];
+ protected $hidden = ['path'];
+ protected $casts = [
+ 'external' => 'bool',
+ ];
/**
* Get the downloadable file name for this upload.
@@ -80,7 +87,7 @@ class Attachment extends Model
/**
* Scope the query to those attachments that are visible based upon related page permissions.
*/
- public function scopeVisible(): string
+ public function scopeVisible(): Builder
{
$permissionService = app()->make(PermissionService::class);
return $permissionService->filterRelatedEntity(
diff --git a/app/Uploads/AttachmentService.php b/app/Uploads/AttachmentService.php
index d530d8fbe..2ad1663ff 100644
--- a/app/Uploads/AttachmentService.php
+++ b/app/Uploads/AttachmentService.php
@@ -162,16 +162,16 @@ class AttachmentService
$link = trim($requestData['link'] ?? '');
if (!empty($link)) {
- $attachment->path = $requestData['link'];
if (!$attachment->external) {
$this->deleteFileInStorage($attachment);
$attachment->external = true;
+ $attachment->extension = '';
}
+ $attachment->path = $requestData['link'];
}
$attachment->save();
-
- return $attachment;
+ return $attachment->refresh();
}
/**
diff --git a/tests/Api/AttachmentsApiTest.php b/tests/Api/AttachmentsApiTest.php
new file mode 100644
index 000000000..88b5b9ddd
--- /dev/null
+++ b/tests/Api/AttachmentsApiTest.php
@@ -0,0 +1,330 @@
+actingAsApiEditor();
+ $page = Page::query()->first();
+ $attachment = $this->createAttachmentForPage($page, [
+ 'name' => 'My test attachment',
+ 'external' => true,
+ ]);
+
+ $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+ $resp->assertJson(['data' => [
+ [
+ 'id' => $attachment->id,
+ 'name' => 'My test attachment',
+ 'uploaded_to' => $page->id,
+ 'external' => true,
+ ],
+ ]]);
+ }
+
+ public function test_attachments_listing_based_upon_page_visibility()
+ {
+ $this->actingAsApiEditor();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $attachment = $this->createAttachmentForPage($page, [
+ 'name' => 'My test attachment',
+ 'external' => true,
+ ]);
+
+ $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+ $resp->assertJson(['data' => [
+ [
+ 'id' => $attachment->id,
+ ],
+ ]]);
+
+ $page->restricted = true;
+ $page->save();
+ $this->regenEntityPermissions($page);
+
+ $resp = $this->getJson($this->baseEndpoint . '?count=1&sort=+id');
+ $resp->assertJsonMissing(['data' => [
+ [
+ 'id' => $attachment->id,
+ ],
+ ]]);
+ }
+
+ public function test_create_endpoint_for_link_attachment()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+
+ $details = [
+ 'name' => 'My attachment',
+ 'uploaded_to' => $page->id,
+ 'link' => 'https://cats.example.com',
+ ];
+
+ $resp = $this->postJson($this->baseEndpoint, $details);
+ $resp->assertStatus(200);
+ /** @var Attachment $newItem */
+ $newItem = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+ $resp->assertJson(['id' => $newItem->id, 'external' => true, 'name' => $details['name'], 'uploaded_to' => $page->id]);
+ }
+
+ public function test_create_endpoint_for_upload_attachment()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $file = $this->getTestFile('textfile.txt');
+
+ $details = [
+ 'name' => 'My attachment',
+ 'uploaded_to' => $page->id,
+ ];
+
+ $resp = $this->call('POST', $this->baseEndpoint, $details, [], ['file' => $file]);
+ $resp->assertStatus(200);
+ /** @var Attachment $newItem */
+ $newItem = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->first();
+ $resp->assertJson(['id' => $newItem->id, 'external' => false, 'extension' => 'txt', 'name' => $details['name'], 'uploaded_to' => $page->id]);
+ $this->assertTrue(file_exists(storage_path($newItem->path)));
+ unlink(storage_path($newItem->path));
+ }
+
+ public function test_name_needed_to_create()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+
+ $details = [
+ 'uploaded_to' => $page->id,
+ 'link' => 'https://example.com',
+ ];
+
+ $resp = $this->postJson($this->baseEndpoint, $details);
+ $resp->assertStatus(422);
+ $resp->assertJson([
+ 'error' => [
+ 'message' => 'The given data was invalid.',
+ 'validation' => [
+ 'name' => ['The name field is required.'],
+ ],
+ 'code' => 422,
+ ],
+ ]);
+ }
+
+ public function test_link_or_file_needed_to_create()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+
+ $details = [
+ 'name' => 'my attachment',
+ 'uploaded_to' => $page->id,
+ ];
+
+ $resp = $this->postJson($this->baseEndpoint, $details);
+ $resp->assertStatus(422);
+ $resp->assertJson([
+ 'error' => [
+ 'message' => 'The given data was invalid.',
+ 'validation' => [
+ "file" => ["The file field is required when link is not present."],
+ "link" => ["The link field is required when file is not present."],
+ ],
+ 'code' => 422,
+ ],
+ ]);
+ }
+
+ public function test_read_endpoint_for_link_attachment()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+
+ $attachment = $this->createAttachmentForPage($page, [
+ 'name' => 'my attachment',
+ 'path' => 'https://example.com',
+ 'order' => 1,
+ ]);
+
+ $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
+
+ $resp->assertStatus(200);
+ $resp->assertJson([
+ 'id' => $attachment->id,
+ 'content' => 'https://example.com',
+ 'external' => true,
+ 'uploaded_to' => $page->id,
+ 'order' => 1,
+ 'created_by' => [
+ 'name' => $attachment->createdBy->name,
+ ],
+ 'updated_by' => [
+ 'name' => $attachment->createdBy->name,
+ ],
+ 'links' => [
+ "html" => "id}\">my attachment",
+ "markdown" => "[my attachment](http://localhost/attachments/{$attachment->id})"
+ ],
+ ]);
+ }
+
+ public function test_read_endpoint_for_file_attachment()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $file = $this->getTestFile('textfile.txt');
+
+ $details = [
+ 'name' => 'My file attachment',
+ 'uploaded_to' => $page->id,
+ ];
+ $this->call('POST', $this->baseEndpoint, $details, [], ['file' => $file]);
+ /** @var Attachment $attachment */
+ $attachment = Attachment::query()->orderByDesc('id')->where('name', '=', $details['name'])->firstOrFail();
+
+ $resp = $this->getJson("{$this->baseEndpoint}/{$attachment->id}");
+
+ $resp->assertStatus(200);
+ $resp->assertJson([
+ 'id' => $attachment->id,
+ 'content' => base64_encode(file_get_contents(storage_path($attachment->path))),
+ 'external' => false,
+ 'uploaded_to' => $page->id,
+ 'order' => 1,
+ 'created_by' => [
+ 'name' => $attachment->createdBy->name,
+ ],
+ 'updated_by' => [
+ 'name' => $attachment->updatedBy->name,
+ ],
+ 'links' => [
+ "html" => "id}\">My file attachment",
+ "markdown" => "[My file attachment](http://localhost/attachments/{$attachment->id})"
+ ],
+ ]);
+
+ unlink(storage_path($attachment->path));
+ }
+
+ public function test_update_endpoint()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $attachment = $this->createAttachmentForPage($page);
+
+ $details = [
+ 'name' => 'My updated API attachment',
+ ];
+
+ $resp = $this->putJson("{$this->baseEndpoint}/{$attachment->id}", $details);
+ $attachment->refresh();
+
+ $resp->assertStatus(200);
+ $resp->assertJson(['id' => $attachment->id, 'name' => 'My updated API attachment']);
+ }
+
+ public function test_update_link_attachment_to_file()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $attachment = $this->createAttachmentForPage($page);
+ $file = $this->getTestFile('textfile.txt');
+
+
+ $resp = $this->call('PUT', "{$this->baseEndpoint}/{$attachment->id}", ['name' => 'My updated file'], [], ['file' => $file]);
+ $resp->assertStatus(200);
+
+ $attachment->refresh();
+ $this->assertFalse($attachment->external);
+ $this->assertEquals('txt', $attachment->extension);
+ $this->assertStringStartsWith('uploads/files/', $attachment->path);
+ $this->assertFileExists(storage_path($attachment->path));
+
+ unlink(storage_path($attachment->path));
+ }
+
+ public function test_update_file_attachment_to_link()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $file = $this->getTestFile('textfile.txt');
+ $this->call('POST', $this->baseEndpoint, ['name' => 'My file attachment', 'uploaded_to' => $page->id], [], ['file' => $file]);
+ /** @var Attachment $attachment */
+ $attachment = Attachment::query()->where('name', '=', 'My file attachment')->firstOrFail();
+
+ $filePath = storage_path($attachment->path);
+ $this->assertFileExists($filePath);
+
+ $details = [
+ 'name' => 'My updated API attachment',
+ 'link' => 'https://cats.example.com'
+ ];
+
+ $resp = $this->putJson("{$this->baseEndpoint}/{$attachment->id}", $details);
+ $resp->assertStatus(200);
+ $attachment->refresh();
+
+ $this->assertFileDoesNotExist($filePath);
+ $this->assertTrue($attachment->external);
+ $this->assertEquals('https://cats.example.com', $attachment->path);
+ $this->assertEquals('', $attachment->extension);
+ }
+
+ public function test_delete_endpoint()
+ {
+ $this->actingAsApiAdmin();
+ /** @var Page $page */
+ $page = Page::query()->first();
+ $attachment = $this->createAttachmentForPage($page);
+
+ $resp = $this->deleteJson("{$this->baseEndpoint}/{$attachment->id}");
+
+ $resp->assertStatus(204);
+ $this->assertDatabaseMissing('attachments', ['id' => $attachment->id]);
+ }
+
+ protected function createAttachmentForPage(Page $page, $attributes = []): Attachment
+ {
+ $admin = $this->getAdmin();
+ /** @var Attachment $attachment */
+ $attachment = $page->attachments()->forceCreate(array_merge([
+ 'uploaded_to' => $page->id,
+ 'name' => 'test attachment',
+ 'external' => true,
+ 'order' => 1,
+ 'created_by' => $admin->id,
+ 'updated_by' => $admin->id,
+ 'path' => 'https://attachment.example.com'
+ ], $attributes));
+ return $attachment;
+ }
+
+ /**
+ * Get a test file that can be uploaded.
+ */
+ protected function getTestFile(string $fileName): UploadedFile
+ {
+ return new UploadedFile(base_path('tests/test-data/test-file.txt'), $fileName, 'text/plain', 55, null, true);
+ }
+}
diff --git a/tests/Api/TestsApi.php b/tests/Api/TestsApi.php
index 683ca0c74..97ca82ea7 100644
--- a/tests/Api/TestsApi.php
+++ b/tests/Api/TestsApi.php
@@ -17,6 +17,16 @@ trait TestsApi
return $this;
}
+ /**
+ * Set the API admin role as the current user via the API driver.
+ */
+ protected function actingAsApiAdmin()
+ {
+ $this->actingAs($this->getAdmin(), 'api');
+
+ return $this;
+ }
+
/**
* Format the given items into a standardised error format.
*/
diff --git a/tests/Uploads/AttachmentTest.php b/tests/Uploads/AttachmentTest.php
index 2248bc2c5..60fd370b6 100644
--- a/tests/Uploads/AttachmentTest.php
+++ b/tests/Uploads/AttachmentTest.php
@@ -76,9 +76,9 @@ class AttachmentTest extends TestCase
$upload->assertStatus(200);
$attachment = Attachment::query()->orderBy('id', 'desc')->first();
- $expectedResp['path'] = $attachment->path;
-
$upload->assertJson($expectedResp);
+
+ $expectedResp['path'] = $attachment->path;
$this->assertDatabaseHas('attachments', $expectedResp);
$this->deleteUploads();