From 2f297e9ad6319cbe1858d5e298b1035779bb2922 Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Mon, 19 Oct 2026 18:56:14 +0100 Subject: [PATCH] feat: attach images to commits from the API The app sends images as multipart, signed the way the devnull commit script signs them: the JSON body goes in a payload part and the signature covers the SHA-256 of each image. The images of a commit that isn't published yet get temporary URLs, so the author can see them in the app, which has no web session. --- KNOWLEDGE_BASE.md | 6 +- README.md | 15 +- app/Http/Controllers/Api/CommitController.php | 2 +- app/Http/Requests/CreateApiCommitRequest.php | 48 ++++++- app/Http/Requests/ShowCommitImageRequest.php | 5 +- app/Http/Resources/CommitResource.php | 9 +- app/Models/CommitImage.php | 11 ++ app/Services/SignedRequests.php | 101 +++++++++++-- tests/Feature/ApiSignatureTest.php | 55 ++++++++ .../Controllers/Api/CommitControllerTest.php | 133 ++++++++++++++++++ .../Controllers/CommitImageControllerTest.php | 22 +++ tests/Pest.php | 24 ++++ tests/Unit/Models/CommitImageTest.php | 17 +++ 13 files changed, 425 insertions(+), 23 deletions(-) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index f6ebb42f9..c333ad71f 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -217,7 +217,7 @@ Sources: [stash hook](resources/js/hooks/use-stash.ts), [image storage](resource ### Images and descriptions -Attach up to **three JPEG, PNG, or WebP images** to a commit or reply. Each image can be at most **10 MB** and **6,000 pixels in either dimension**. Add an image description of up to **1,000 characters** for accessibility. Add images with the picker, by pasting, or by dropping them on the composer. Every image is re-encoded to WebP, at most 1,600 pixels wide, which strips metadata such as location. Images can be posted without accompanying text. Images on a pending commit are visible only to its author and admins. Image attachments and a poll cannot be combined in the same commit. Open an attached photo to view it in the photo viewer, move through the commit's images, read its description, or download it. Terminal commits can attach images without descriptions; the signed API cannot attach images. +Attach up to **three JPEG, PNG, or WebP images** to a commit or reply. Each image can be at most **10 MB** and **6,000 pixels in either dimension**. Add an image description of up to **1,000 characters** for accessibility. Add images with the picker, by pasting, or by dropping them on the composer. Every image is re-encoded to WebP, at most 1,600 pixels wide, which strips metadata such as location. Images can be posted without accompanying text. Images on a pending commit are visible only to its author and admins. Image attachments and a poll cannot be combined in the same commit. Open an attached photo to view it in the photo viewer, move through the commit's images, read its description, or download it. Terminal commits can attach images without descriptions, and signed API commits with them. Sources: [image picker](resources/js/components/commit-image-picker.tsx), [image limits](app/Actions/CreateCommitImages.php), [attachment validation](app/Http/Requests/CreateCommitRequest.php), [photo controller](app/Http/Controllers/CommitPhotoController.php). @@ -430,7 +430,7 @@ An app can use the same `/api/v1` for you with signed requests. It keeps an SSH A signed request sees what the web shows you, such as your own pending commits. It can also, with the web's validation, moderation, and rate limits: - read your account, logs, replies, notifications, and saved commits; -- commit, reply, and quote, and delete your commits; +- commit, reply, and quote, with images, and delete your commits; - follow or unfollow, mute or unmute, and block or unblock, and list who you muted and blocked; - star or unstar, and save or unsave; - switch Pastéis mode and give pastéis; @@ -438,7 +438,7 @@ A signed request sees what the web shows you, such as your own pending commits. - sync from GitHub and save your lane color; - delete your account. -Signed request bodies are JSON, so app commits are text only: no images, polls, or link-card choice. Pinning, Remotes, scheduling, search, and translations are web-only for now. Signed requests are limited to **120 requests per minute per user**. Settings → Devices lists your approved devices with when each was last used, and revokes them. +Signed request bodies are JSON, or multipart when they carry images, so app commits can have images but no polls or link-card choice. Pinning, Remotes, scheduling, search, and translations are web-only for now. Signed requests are limited to **120 requests per minute per user**. Settings → Devices lists your approved devices with when each was last used, and revokes them. Sources: [signed requests](app/Services/SignedRequests.php), [sign-in page](app/Http/Controllers/MobileAuthorizationController.php), [devices settings](resources/js/pages/device/index.tsx), [API routes](routes/api.php). diff --git a/README.md b/README.md index 94e195109..29ad089aa 100644 --- a/README.md +++ b/README.md @@ -93,6 +93,8 @@ A signed profile also says `is_muting` and has the web's `achievements`, with th `POST /api/v1/commits` takes a `body`, and either a `parent_id` to reply to a commit or a `quoted_commit_id` to quote one, but not both. Returns a `201` on successful commit creation. +A commit can also carry up to 3 images, as on the web: JPEG, PNG or WebP, at most 10 MB and 6000 pixels a side each, with an optional `image_descriptions` list in the same order, up to 1000 characters each. With images, the body may be short or left out. They are sent as multipart, signed as shown under the headers below. The images of the user's own commits that aren't published yet come with a temporary `url` that works without a session for at least an hour, since the app has none. + `POST /api/v1/commits/{id}/reports` reports a commit to the moderators, privately, as the web's Report button does. It takes a `reason`, one of `spam`, `harassment`, `unsafe` or `other`, and optional `details` of up to 500 characters, and answers `204`. Reporting doesn't hide the commit. Only someone else's published commit can be reported: the user's own is a `403` with `own_commit`, and one that is pending, rejected, deleted, missing or across a block is a `404` with `commit_unavailable`. Reporting a commit again also answers `204` and keeps the first report. Each user can send 5 reports a minute. `GET /api/v1/me` also has what only the user sees in the web's profile settings: `preferred_lane` (a lane, a custom `#rrggbb` color, or `null` for Automatic), the `automatic_lane` Automatic picks for them, their `sponsors_url` (or `null`), and `github_synced_at`, when their profile was last synced from GitHub (`null` if it only was when they signed in). `PUT /api/v1/me/github` syncs it from GitHub again, like Sync from GitHub on the web, and shares its limit of one sync a minute. `PUT /api/v1/me/lane` saves their color, with a `lane` from `0` to `5`, or `null` for Automatic, or a custom `color` as lowercase `#rrggbb` with a `null` lane, as the web's color picker does. Both answer like `GET /api/v1/me`. @@ -107,7 +109,7 @@ Each request sends three headers: | `X-Devnull-Nonce` | 32 random hex characters, used once | | `X-SSH-Signature` | The SSHSIG signature, base64 on one line | -The signature is made with `ssh-keygen -Y sign -n devnull-api` over these lines, joined by `\n`: the timestamp, the nonce, the method, the path with its query string exactly as sent, and the raw body. A body goes as JSON, with `Content-Type: application/json`, since the signature covers its bytes but not how they are read. Any other body is refused. +The signature is made with `ssh-keygen -Y sign -n devnull-api` over these lines, joined by `\n`: the timestamp, the nonce, the method, the path with its query string exactly as sent, and the raw body. A body goes as JSON, with `Content-Type: application/json`, since the signature covers its bytes but not how they are read. Any other body is refused, except for a request with images, below. ```text 1760534400 @@ -117,6 +119,17 @@ POST {"body":"Hello from my phone"} ``` +A request with images is sent as `multipart/form-data` instead, like a commit with images from `devnull commit`: the JSON body goes in a `payload` file part, the images in `images[]`, and nothing else. It is signed with `-n devnull-api-images`, with one more line before the body: the SHA-256 of each image, in hex, comma-separated in the order they are attached. + +```text +1760534400 +3f9c2a7e5b1d4c8f9a0b2c3d4e5f6a7b +POST +/api/v1/commits +9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08,60303ae22b998861bce3b28f33eec1be758a213c86c93c076dbe9f558c11c752 +{"body":"Shipped it","image_descriptions":["The new settings page",null]} +``` + A request with any of these headers has to be signed right. A bad or expired signature is a `401`, never the public view. A request without them always gets the public view, and across a block a signed request gets neither profile: it is a `403`, `user_blocked` for the one who blocked and `user_unavailable` for the other, and their commits are hidden as on the web. A key signs in once its user approved it as a device. It doesn't have to be on their GitHub account, and being on it isn't enough, so a key made for the GitHub Action can't use the API. Users see their devices, when each was last used, and revoke them under Settings → Devices (`/settings/devices`). A key that isn't approved, or was revoked, gets a `401` with `device_not_approved`, and the app signs in again. diff --git a/app/Http/Controllers/Api/CommitController.php b/app/Http/Controllers/Api/CommitController.php index 305287228..97ea97aa4 100644 --- a/app/Http/Controllers/Api/CommitController.php +++ b/app/Http/Controllers/Api/CommitController.php @@ -45,7 +45,7 @@ public function show(ShowApiCommitRequest $request, #[CurrentUser] ?User $curren */ public function store(CreateApiCommitRequest $request, #[CurrentUser] User $user, CreateCommit $action, CommitPageQuery $page): JsonResponse { - $commit = $action->handle($user, $request->string('body')->value(), $request->parent(), $request->quotedCommit()); + $commit = $action->handle($user, $request->string('body')->value(), $request->parent(), $request->quotedCommit(), $request->images(), $request->imageDescriptions()); return CommitResource::make($page->get($commit, $user)) ->response() diff --git a/app/Http/Requests/CreateApiCommitRequest.php b/app/Http/Requests/CreateApiCommitRequest.php index 8cf417531..c0a9fbf74 100644 --- a/app/Http/Requests/CreateApiCommitRequest.php +++ b/app/Http/Requests/CreateApiCommitRequest.php @@ -9,10 +9,14 @@ use App\Rules\VisibleCommit; use Illuminate\Contracts\Validation\ValidationRule; use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Http\UploadedFile; +use Illuminate\Validation\Rules\Dimensions; +use Illuminate\Validation\Rules\Enum; +use Illuminate\Validation\Rules\File; /** - * A commit from the mobile app, sent as JSON. It may reply to a commit or quote one, as on the web, but not both. It has - * no images or poll yet. + * A commit from the mobile app, sent as JSON, or with images as multipart (see SignedRequests). It may reply to a commit + * or quote one, as on the web, but not both. The images are checked as the web checks them. It has no poll yet. */ final class CreateApiCommitRequest extends FormRequest { @@ -30,17 +34,32 @@ public function prepareForValidation(): void } /** - * @return array> + * @return array> */ public function rules(): array { $user = $this->user(); assert($user instanceof User); + $attachmentRules = CreateCommitRequest::attachmentRules(); return [ - 'body' => CreateCommitRequest::bodyRules(), + 'body' => CreateCommitRequest::bodyRules($this->hasFile('images')), 'parent_id' => ['bail', 'nullable', 'uuid', new VisibleCommit($user)], 'quoted_commit_id' => ['bail', 'nullable', 'uuid', 'prohibits:parent_id', new VisibleCommit($user, __('That commit is no longer available to quote.'))], + 'images' => $attachmentRules['images'], + 'images.*' => $attachmentRules['images.*'], + 'image_descriptions' => $attachmentRules['image_descriptions'], + 'image_descriptions.*' => $attachmentRules['image_descriptions.*'], + ]; + } + + /** + * @return array + */ + public function attributes(): array + { + return [ + 'image_descriptions.*' => 'image description', ]; } @@ -66,4 +85,25 @@ public function quotedCommit(): ?Commit ? Commit::query()->visibleTo($user)->find($this->string('quoted_commit_id')->value()) : null; } + + /** + * @return list + */ + public function images(): array + { + $images = $this->file('images'); + + return is_array($images) ? array_values($images) : []; + } + + /** + * @return list + */ + public function imageDescriptions(): array + { + /** @var list $descriptions */ + $descriptions = $this->array('image_descriptions'); + + return $descriptions; + } } diff --git a/app/Http/Requests/ShowCommitImageRequest.php b/app/Http/Requests/ShowCommitImageRequest.php index 67f602544..218e2f9f0 100644 --- a/app/Http/Requests/ShowCommitImageRequest.php +++ b/app/Http/Requests/ShowCommitImageRequest.php @@ -13,7 +13,8 @@ final class ShowCommitImageRequest extends FormRequest { /** - * Pending and rejected images never leave the server for anyone but their author and moderators. + * Pending and rejected images never leave the server for anyone but their author and moderators, or the temporary + * URL the API gave one of them, until the commit is deleted. */ public function authorize(): Response { @@ -22,7 +23,7 @@ public function authorize(): Response $commit = Commit::query()->find($commitImage->commit_id); - return $commit instanceof Commit && $commit->imagesAreVisibleTo($this->user()) && Storage::disk()->exists($commitImage->path) + return $commit instanceof Commit && ($commit->imagesAreVisibleTo($this->user()) || (! $commit->trashed() && $this->hasValidSignature())) && Storage::disk()->exists($commitImage->path) ? Response::allow() : Response::denyAsNotFound(); } diff --git a/app/Http/Resources/CommitResource.php b/app/Http/Resources/CommitResource.php index 22f1c4927..6ce3b53b2 100644 --- a/app/Http/Resources/CommitResource.php +++ b/app/Http/Resources/CommitResource.php @@ -4,7 +4,9 @@ namespace App\Http\Resources; +use App\Enums\CommitStatus; use App\Models\Commit; +use App\Models\CommitImage; use App\Models\Poll; use App\Models\User; use App\Services\CommitBodies; @@ -24,7 +26,8 @@ * its votes, and the commit it quotes, or null once the user may no longer see that one, while quoted_commit_id still * says it quoted something. In a list, reply_preview is the newest reply or quote the web shows under it. As on the * web, the counts, link_preview, images, starred, parent, poll, quoted_commit and reply_preview are only - * there when they were loaded, so ancestors and the commits of notifications come without some of them. + * there when they were loaded, so ancestors and the commits of notifications come without some of them. The images of + * a commit that isn't published yet have temporary URLs, since the app has no web session to load them with. * * A deleted commit that still has replies stays in its thread as a tombstone: its id, parent_id, deleted_at and * replies_count, but never its author or body. Only signed requests are ever sent one. @@ -59,7 +62,9 @@ public function toArray(Request $request): array 'stars_count' => $this->whenCounted('stars'), 'replies_count' => $this->whenCounted('replies'), 'link_preview' => $this->whenLoaded('linkPreview'), - 'images' => $this->whenLoaded('images'), + 'images' => $this->whenLoaded('images', fn (): mixed => $this->status === CommitStatus::Approved || ! $this->imagesAreVisibleTo($request->user()) + ? $this->images + : $this->images->map(fn (CommitImage $image): array => [...$image->toArray(), 'url' => $image->temporaryUrl()])), 'author' => UserResource::make($this->user), ]; diff --git a/app/Models/CommitImage.php b/app/Models/CommitImage.php index 611dd8d4e..c432140ba 100644 --- a/app/Models/CommitImage.php +++ b/app/Models/CommitImage.php @@ -11,6 +11,7 @@ use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use Illuminate\Support\Facades\URL; /** * @property-read string $id @@ -78,4 +79,14 @@ public function url(): string { return route('commit-image.show', $this); } + + /** + * Serves the image to anyone who has the URL, until it expires. The API hands it to the author of a pending commit, + * since the mobile app has no web session to load their own pending images with. It stays the same for an hour at + * a time, so the app can keep the image it loaded. + */ + public function temporaryUrl(): string + { + return URL::temporarySignedRoute('commit-image.show', now()->startOfHour()->addHours(2), ['commitImage' => $this]); + } } diff --git a/app/Services/SignedRequests.php b/app/Services/SignedRequests.php index f21a69413..b63040bc9 100644 --- a/app/Services/SignedRequests.php +++ b/app/Services/SignedRequests.php @@ -7,8 +7,12 @@ use App\Actions\UpdateDeviceLastUsedAt; use App\Dto\SshPublicKey; use App\Models\Device; +use Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull; +use Illuminate\Foundation\Http\Middleware\TrimStrings; use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; use InvalidArgumentException; +use Symfony\Component\HttpFoundation\File\UploadedFile; /** * Verifies a request to the API, signed by the mobile app with an SSH key it keeps on the device. @@ -16,6 +20,11 @@ * The signed message is "\n\n\n\n". The timestamp and nonce * are also sent in the X-Devnull-Timestamp and X-Devnull-Nonce headers, and the signature in X-SSH-Signature. * + * A request with images is sent as multipart, like a commit with images from the `devnull commit` script: the JSON + * body goes in a payload part, the images in images[], and the signed message carries one more line before the body, + * the comma-separated SHA-256 of each image in the order they are attached. It is signed under its own namespace, so + * the signature states which format it covers. + * * A key signs in as a user once that user approved it as a device on the web. The key doesn't have to be on their * GitHub account: the approval is the proof. So a key made for something else, like committing from CI, can't sign in. */ @@ -23,6 +32,8 @@ { public const string Namespace = 'devnull-api'; + public const string ImagesNamespace = 'devnull-api-images'; + private const array Headers = ['X-Devnull-Timestamp', 'X-Devnull-Nonce', 'X-SSH-Signature']; public function __construct(private SshSignatures $sshSignatures, private UpdateDeviceLastUsedAt $updateDeviceLastUsedAt) @@ -40,29 +51,31 @@ public function isSigned(Request $request): bool /** * Checks the signature, its timestamp and its nonce, which it spends, and returns the key that signed the request, - * whether or not it is approved as a device yet. + * whether or not it is approved as a device yet. A request with images reads its fields from its payload from + * then on. */ public function verifySignature(Request $request): SshPublicKey { - // Only the raw body is signed, so it has to be JSON. PHP hands a multipart body over as fields and files and - // keeps no raw body, and the same signed bytes read as a form would give the endpoint other fields... - throw_if( - $request->files->count() > 0 || ($request->getContent() === '' ? $request->request->count() > 0 : ! $request->isJson()), - InvalidArgumentException::class, - 'Send the body of a signed request as JSON.', - ); + $withImages = $request->files->count() > 0; + $body = $withImages ? $this->payload($request) : $this->body($request); + + throw_if($body === null, InvalidArgumentException::class, 'Send the body of a signed request as JSON.'); $timestamp = $request->headers->get('X-Devnull-Timestamp', '') ?? ''; $nonce = $request->headers->get('X-Devnull-Nonce', '') ?? ''; $publicKey = $this->sshSignatures->verify( - implode("\n", [$timestamp, $nonce, $request->getMethod(), $request->getRequestUri(), $request->getContent()]), + implode("\n", [$timestamp, $nonce, $request->getMethod(), $request->getRequestUri(), ...($withImages ? [implode(',', $this->imageHashes($request))] : []), $body]), $request->headers->get('X-SSH-Signature', '') ?? '', - self::Namespace, + $withImages ? self::ImagesNamespace : self::Namespace, ); $this->sshSignatures->checkFreshness($timestamp, $nonce); + if ($withImages) { + $this->readPayload($request, $body); + } + return $publicKey; } @@ -86,4 +99,72 @@ public function verify(Request $request): array 'device' => $device, ]; } + + /** + * The raw body, which is all that is signed, so it has to be JSON. PHP hands a multipart body over as fields and + * files and keeps no raw body, and the same signed bytes read as a form would give the endpoint other fields. + */ + private function body(Request $request): ?string + { + $content = $request->getContent(); + + if ($content === '') { + return $request->request->count() > 0 ? null : ''; + } + + return $request->isJson() ? $content : null; + } + + /** + * The JSON body of a request with images, or null unless it came as a payload part beside a list of images, with + * no other fields or files. + */ + private function payload(Request $request): ?string + { + $files = $request->files->all(); + $payload = $files['payload'] ?? null; + $images = $files['images'] ?? null; + + $isMultipart = $request->request->count() === 0 + && count($files) === 2 + && $payload instanceof UploadedFile && $payload->isValid() + && is_array($images) && array_is_list($images) && array_all($images, fn (mixed $image): bool => $image instanceof UploadedFile); + + $contents = $isMultipart ? file_get_contents($payload->getPathname()) : false; + + return is_string($contents) && is_array(json_decode($contents, true)) ? $contents : null; + } + + /** + * An image PHP couldn't take, too big for its upload limit for one, can't be hashed, and is a validation error, + * as on the web, rather than a bad signature. + * + * @return list + */ + private function imageHashes(Request $request): array + { + /** @var list $images */ + $images = $request->files->all()['images']; + + return array_map(function (UploadedFile $image): string { + throw_unless($image->isValid(), ValidationException::withMessages(['images' => __('validation.uploaded', ['attribute' => 'image'])])); + + return (string) hash_file('sha256', $image->getPathname()); + }, $images); + } + + /** + * The payload becomes the request's fields, cleaned the way every request's fields are before it gets here. + */ + private function readPayload(Request $request, string $payload): void + { + /** @var array $fields */ + $fields = json_decode($payload, true); + + $request->request->replace($fields); + + foreach ([TrimStrings::class, ConvertEmptyStringsToNull::class] as $middleware) { + resolve($middleware)->handle($request, fn (Request $request): Request => $request); + } + } } diff --git a/tests/Feature/ApiSignatureTest.php b/tests/Feature/ApiSignatureTest.php index ef2d6a115..01d0f472d 100644 --- a/tests/Feature/ApiSignatureTest.php +++ b/tests/Feature/ApiSignatureTest.php @@ -7,6 +7,7 @@ use App\Models\Device; use App\Models\SshKey; use App\Models\User; +use App\Services\SignedRequests; use Carbon\CarbonImmutable; use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Auth; @@ -316,6 +317,60 @@ expect(Commit::query()->exists())->toBeFalse(); }); +it('rejects images that do not match the hashes that were signed', function (Closure $signed): void { + $images = [UploadedFile::fake()->image('wide.png', 40, 10), UploadedFile::fake()->image('tall.png', 10, 40)]; + + signedApiRequestWithImages($this->keyPair, 'POST', route('api-commit.store'), ['body' => 'It was DNS.'], $images, $signed($images)) + ->assertUnauthorized() + ->assertExactJson(['message' => 'Invalid signature.', 'code' => 'invalid_signature']); + + expect(Commit::query()->exists())->toBeFalse(); +})->with([ + 'another image' => fn (array $images): array => [$images[0], UploadedFile::fake()->image('other.png', 10, 10)], + 'another order' => array_reverse(...), + 'one image less' => fn (array $images): array => [$images[0]], +]); + +it('rejects images signed as a json body, without their own namespace', function (): void { + signedApiRequestWithImages($this->keyPair, 'POST', route('api-commit.store'), ['body' => 'It was DNS.'], [UploadedFile::fake()->image('dns.png', 10, 10)], namespace: SignedRequests::Namespace) + ->assertUnauthorized() + ->assertExactJson(['message' => 'Invalid signature.', 'code' => 'invalid_signature']); + + expect(Commit::query()->exists())->toBeFalse(); +}); + +it('rejects images sent any other way than beside a json payload', function (array $parameters, Closure $files): void { + $image = UploadedFile::fake()->image('dns.png', 10, 10); + $payload = '{"body":"It was DNS."}'; + $server = [ + ...signApiRequest($this->keyPair, 'POST', route('api-commit.store'), hash_file('sha256', $image->getPathname())."\n".$payload, namespace: SignedRequests::ImagesNamespace), + 'CONTENT_TYPE' => 'multipart/form-data; boundary=devnull', + ]; + + $this->call('POST', route('api-commit.store'), $parameters, files: $files($image, $payload), server: $server) + ->assertUnauthorized() + ->assertExactJson(['message' => 'Send the body of a signed request as JSON.', 'code' => 'invalid_signature']); + + expect(Commit::query()->exists())->toBeFalse(); +})->with([ + 'with a field beside the payload' => [['body' => 'Not what was signed'], fn (UploadedFile $image, string $payload): array => ['payload' => UploadedFile::fake()->createWithContent('payload', $payload), 'images' => [$image]]], + 'with the payload as a field' => [['payload' => '{"body":"It was DNS."}'], fn (UploadedFile $image, string $payload): array => ['images' => [$image]]], + 'with a payload that is not json' => [[], fn (UploadedFile $image, string $payload): array => ['payload' => UploadedFile::fake()->createWithContent('payload', 'body=It was DNS.'), 'images' => [$image]]], + 'with another file' => [[], fn (UploadedFile $image, string $payload): array => ['payload' => UploadedFile::fake()->createWithContent('payload', $payload), 'images' => [$image], 'avatar' => $image]], + 'with one image that is not in a list' => [[], fn (UploadedFile $image, string $payload): array => ['payload' => UploadedFile::fake()->createWithContent('payload', $payload), 'images' => $image]], + 'with nested images' => [[], fn (UploadedFile $image, string $payload): array => ['payload' => UploadedFile::fake()->createWithContent('payload', $payload), 'images' => [[$image]]]], +]); + +it('says when an image did not upload, which is no fault of the signature', function (): void { + $image = UploadedFile::fake()->image('dns.png', 10, 10); + + signedApiRequestWithImages($this->keyPair, 'POST', route('api-commit.store'), ['body' => 'It was DNS.'], [new UploadedFile($image->getPathname(), 'dns.png', 'image/png', UPLOAD_ERR_INI_SIZE, test: true)], [$image]) + ->assertUnprocessable() + ->assertJsonValidationErrors(['images' => 'The image failed to upload.']); + + expect(Commit::query()->exists())->toBeFalse(); +}); + it('limits each address to 60 failed signatures a minute, before checking any more', function (): void { $stranger = SshKeyPair::generate(); $sign = fn (SshKeyPair $keyPair, string $address): array => [...signApiRequest($keyPair, 'GET', route('api-current-user.show')), 'REMOTE_ADDR' => $address]; diff --git a/tests/Feature/Controllers/Api/CommitControllerTest.php b/tests/Feature/Controllers/Api/CommitControllerTest.php index 78ef4fdae..ffeeae9a6 100644 --- a/tests/Feature/Controllers/Api/CommitControllerTest.php +++ b/tests/Feature/Controllers/Api/CommitControllerTest.php @@ -12,7 +12,10 @@ use App\Models\PollOption; use App\Models\PollVote; use App\Models\User; +use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Queue; +use Illuminate\Support\Facades\Storage; it('shows an approved commit to anyone', function (): void { $user = User::factory()->create([ @@ -655,6 +658,136 @@ expect($quoted->quotes()->count())->toBe(0); })->with(['blocking' => true, 'blocked' => false]); +it('commits images from a signed request, in order, with their descriptions', function (): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + + $response = signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), [ + 'body' => 'Offsite photo dump', + 'image_descriptions' => ['The cabin at sunrise, fog over the lake.', null], + ], [ + UploadedFile::fake()->image('cabin.jpg', 40, 30), + UploadedFile::fake()->image('hike.png', 10, 20), + UploadedFile::fake()->image('lake.webp', 10, 10), + ]); + + $commit = $user->commits()->sole(); + $images = $commit->images()->orderBy('position')->get(); + + $response->assertCreated() + ->assertJsonPath('data.body', 'Offsite photo dump') + ->assertJsonPath('data.status', 'pending') + ->assertJsonPath('data.images', $images->map(fn (CommitImage $image): array => [...$image->toArray(), 'url' => $image->temporaryUrl()])->all()); + + expect($images->map(fn (CommitImage $image): array => [$image->width, $image->height, $image->description])->all())->toBe([ + [40, 30, 'The cabin at sunrise, fog over the lake.'], + [10, 20, null], + [10, 10, null], + ])->and(Storage::disk()->allFiles('commit-images'))->toHaveCount(3); +}); + +it('may commit images from a signed request with a short body, or none, as on the web', function (array $body, string $expected): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + + signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), $body, [UploadedFile::fake()->image('screenshot.png', 40, 30)]) + ->assertCreated(); + + expect($user->commits()->sole()) + ->body->toBe($expected) + ->images->toHaveCount(1); +})->with([ + 'missing' => [[], ''], + 'blank' => [['body' => ' '], ''], + 'short' => [['body' => 'ok'], 'ok'], +]); + +it('reads the fields of a commit with images from its payload, cleaned like a json body', function (): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + + signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), [ + 'body' => ' It was DNS :tada: ', + 'image_descriptions' => [' ', ''], + ], [ + UploadedFile::fake()->image('first.jpg', 10, 10), + UploadedFile::fake()->image('second.jpg', 10, 10), + ])->assertCreated(); + + $commit = $user->commits()->sole(); + + expect($commit->body)->toBe('It was DNS 🎉') + ->and($commit->images->pluck('description')->all())->toBe([null, null]); +}); + +it('replies and quotes with images from a signed request', function (string $field): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + $other = Commit::factory()->create(); + + signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), [$field => $other->id], [UploadedFile::fake()->image('trace.png', 10, 10)]) + ->assertCreated() + ->assertJsonPath("data.{$field}", $other->id) + ->assertJsonCount(1, 'data.images'); +})->with(['parent_id', 'quoted_commit_id']); + +it('validates the images of a commit from a signed request as the web does', function (Closure $images, array $data, string $field, string $message): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + + signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), ['body' => 'Look at this', ...$data], $images()) + ->assertUnprocessable() + ->assertJsonValidationErrors([$field => $message]); + + expect($user->commits()->exists())->toBeFalse() + ->and(Storage::disk()->allFiles())->toBe([]); +})->with([ + 'more than three' => [fn (): array => array_map(fn (int $index): UploadedFile => UploadedFile::fake()->image("{$index}.png", 10, 10), range(1, 4)), [], 'images', 'The images field must not have more than 3 items.'], + 'not an image' => [fn (): array => [UploadedFile::fake()->create('notes.txt', 1, 'text/plain')], [], 'images.0', 'The images.0 field must be an image.'], + 'a gif' => [fn (): array => [UploadedFile::fake()->image('party.gif', 10, 10)], [], 'images.0', 'The images.0 field must be a file of type: image/jpeg, image/png, image/webp.'], + 'larger than 10 MB' => [fn (): array => [UploadedFile::fake()->image('huge.png', 10, 10)->size(10_001)], [], 'images.0', 'The images.0 field must not be greater than 10000 kilobytes.'], + 'too many pixels' => [fn (): array => [UploadedFile::fake()->image('wide.png', 6001, 1)], [], 'images.0', 'The images.0 field has invalid image dimensions.'], + 'too long a description' => [fn (): array => [UploadedFile::fake()->image('photo.jpg', 10, 10)], ['image_descriptions' => [str_repeat('a', 1001)]], 'image_descriptions.0', 'The image description field must not be greater than 1000 characters.'], +]); + +it('shows the author the images of their pending commit through urls that need no session', function (): void { + Storage::fake(); + + $user = User::factory()->create(); + $keyPair = apiKeyPairFor($user); + + signedApiRequestWithImages($keyPair, 'POST', route('api-commit.store'), ['body' => 'Look at this'], [UploadedFile::fake()->image('screenshot.png', 10, 10)]) + ->assertCreated(); + + $commit = $user->commits()->sole(); + $image = $commit->images->sole(); + + $url = signedApiRequest($keyPair, 'GET', route('api-commit.show', $commit)) + ->assertOk() + ->assertJsonPath('data.images.0.url', $image->temporaryUrl()) + ->json('data.images.0.url'); + + Auth::forgetGuards(); + + $this->get($url)->assertOk()->assertHeader('Content-Type', 'image/webp'); + + $commit->update(['status' => CommitStatus::Approved]); + + signedApiRequest($keyPair, 'GET', route('api-commit.show', $commit)) + ->assertOk() + ->assertJsonPath('data.images.0.url', route('commit-image.show', $image)); +}); + it('sends a commit from a signed request to the ai moderation the web uses', function (): void { config(['services.openai.key' => 'sk-test']); Queue::fake(); diff --git a/tests/Feature/Controllers/CommitImageControllerTest.php b/tests/Feature/Controllers/CommitImageControllerTest.php index f8cd2ae8c..40de2d813 100644 --- a/tests/Feature/Controllers/CommitImageControllerTest.php +++ b/tests/Feature/Controllers/CommitImageControllerTest.php @@ -88,6 +88,28 @@ function storedCommitImage(Commit $commit): CommitImage ->assertOk(); }); +it('serves images of pending and rejected commits through a temporary url, until it expires', function (string $state): void { + $image = storedCommitImage(Commit::factory()->{$state}()->create()); + $url = $image->temporaryUrl(); + + $this->get($url)->assertOk(); + $this->get($url.'&download=1')->assertNotFound(); + + $this->travel(2)->hours(); + + $this->get($url)->assertNotFound(); +})->with(['pending', 'rejected']); + +it('hides images of deleted commits behind a temporary url', function (): void { + $commit = Commit::factory()->pending()->create(); + $image = storedCommitImage($commit); + $url = $image->temporaryUrl(); + + $commit->delete(); + + $this->get($url)->assertNotFound(); +}); + it('hides images of deleted commits', function (): void { $commit = Commit::factory()->create(); $image = storedCommitImage($commit); diff --git a/tests/Pest.php b/tests/Pest.php index 6e10bd963..2a5c8d897 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -9,8 +9,10 @@ use App\Models\User; use App\Services\OpenAI; use App\Services\SignedCommits; +use App\Services\SignedRequests; use Carbon\CarbonInterface; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Process; @@ -322,6 +324,28 @@ function signedApiRequest(SshKeyPair $keyPair, string $method, string $uri, ?arr return test()->call($method, $uri, server: signApiRequest($keyPair, $method, $uri, $body, $timestamp, $nonce, $namespace), content: $body); } +/** + * Sends an API request with images the way the mobile app does: multipart, with the data as the JSON payload beside the + * images, signed with the hashes of $signedImages, which are the images sent unless a test says otherwise. + * + * @param array $data + * @param list $images + * @param list|null $signedImages + */ +function signedApiRequestWithImages(SshKeyPair $keyPair, string $method, string $uri, array $data, array $images, ?array $signedImages = null, string $namespace = SignedRequests::ImagesNamespace): TestResponse +{ + $payload = json_encode($data, JSON_THROW_ON_ERROR); + $hashes = implode(',', array_map(fn (UploadedFile $image): string => (string) hash_file('sha256', $image->getPathname()), $signedImages ?? $images)); + + return test()->call($method, $uri, files: [ + 'payload' => UploadedFile::fake()->createWithContent('payload', $payload), + 'images' => $images, + ], server: [ + ...signApiRequest($keyPair, $method, $uri, $hashes."\n".$payload, namespace: $namespace), + 'CONTENT_TYPE' => 'multipart/form-data; boundary=devnull', + ]); +} + /** * Gives the user a key approved as a device, that the mobile app signs API requests with. It isn't on their GitHub * account, as the API doesn't need it to be. diff --git a/tests/Unit/Models/CommitImageTest.php b/tests/Unit/Models/CommitImageTest.php index 5cb44abac..ef41fddca 100644 --- a/tests/Unit/Models/CommitImageTest.php +++ b/tests/Unit/Models/CommitImageTest.php @@ -31,3 +31,20 @@ expect(CommitImage::query()->count())->toBe(0); }); + +it('has a temporary url that stays the same for an hour at a time', function (): void { + $this->travelTo(now()->setTime(9, 0)); + + $image = CommitImage::factory()->create(); + $url = $image->temporaryUrl(); + + expect($url)->toStartWith(route('commit-image.show', $image).'?expires='.now()->setTime(11, 0)->getTimestamp().'&signature='); + + $this->travelTo(now()->setTime(9, 59, 59)); + + expect($image->temporaryUrl())->toBe($url); + + $this->travelTo(now()->setTime(10, 0)); + + expect($image->temporaryUrl())->not->toBe($url); +});