diff --git a/app/Console/Commands/FindSusFiles.php b/app/Console/Commands/FindSusFiles.php index bd1652261..6a470f7d7 100644 --- a/app/Console/Commands/FindSusFiles.php +++ b/app/Console/Commands/FindSusFiles.php @@ -36,7 +36,11 @@ public function handle(AttachmentService $service): void SusFiles::query()->delete(); $this->withProgressBar( - Storage::disk('attachments')->allFiles('projects'), + // quarantined (BAD) files keep the projects/ layout under the quarantine prefix + array_merge( + Storage::disk('attachments')->allFiles('projects'), + Storage::disk('attachments')->allFiles(AttachmentService::QUARANTINE_DIR . '/projects'), + ), function ($filePath) use ($service) { $fileName = Str::of($filePath)->basename()->toString(); diff --git a/app/Console/Commands/RotateScreenshots.php b/app/Console/Commands/RotateScreenshots.php index ba1eb1443..d78eba1a0 100644 --- a/app/Console/Commands/RotateScreenshots.php +++ b/app/Console/Commands/RotateScreenshots.php @@ -31,6 +31,11 @@ class RotateScreenshots extends Command */ public function handle(): void { + if (!StorageCleaner::isSupported()) { + $this->info('Screenshots are on object storage: use bucket lifecycle rules instead of rotation'); + return; + } + if (!StorageCleaner::needThinning()) { $this->info('Time for thinning hasn\'t come'); return; diff --git a/app/Contracts/AttachmentService.php b/app/Contracts/AttachmentService.php index 44d18fb8d..568e554b0 100644 --- a/app/Contracts/AttachmentService.php +++ b/app/Contracts/AttachmentService.php @@ -5,10 +5,21 @@ use App\Models\Attachment; use App\Models\Project; use App\Models\Task; +use DateTimeInterface; use Illuminate\Http\UploadedFile; interface AttachmentService { + /** Longest lifetime of a temporary download link: 7 days, the S3 (SigV4) presigning limit */ + public const MAX_TEMPORARY_URL_SECONDS = 7 * 24 * 60 * 60; + + /** + * BAD attachments are moved under this prefix of the attachments disk (quarantine/projects//): + * the file is kept for inspection, while the public key, and every presigned URL issued for it while the + * attachment was GOOD, stops resolving. + */ + public const QUARANTINE_DIR = 'quarantine'; + public function storeFile(UploadedFile $file, Attachment $attachment): string|false; public function handleProjectChange(Task $parent): void; @@ -29,10 +40,18 @@ public function attach(AttachmentAble $parent, array $idsToAttach); public function getProjectPath(Project $project): string; + /** Storage key of the file; quarantine/projects/... for BAD attachments */ public function getPath(Attachment $attachment): string; public function getFullPath(Attachment $attachment): string; + /** + * Presigned download URL when attachments are on object storage (S3); null for the local disk + * (the signed attachment.temporary-download route streams the file then). + * The expiration must not exceed MAX_TEMPORARY_URL_SECONDS. + */ + public function getTemporaryUrl(Attachment $attachment, DateTimeInterface $expiresAt): ?string; + public function getHashAlgo(): string; public function getHashSum(Attachment|string $attachment): false|string; diff --git a/app/Contracts/ScreenshotService.php b/app/Contracts/ScreenshotService.php index ca106a721..2c98008db 100644 --- a/app/Contracts/ScreenshotService.php +++ b/app/Contracts/ScreenshotService.php @@ -2,11 +2,13 @@ namespace App\Contracts; +use App\Helpers\StorageHelper; use App\Jobs\GenerateScreenshotThumbnail; use App\Models\TimeInterval; use Intervention\Image\ImageManagerStatic as Image; use Intervention\Image\Constraint; use Storage; +use URL; abstract class ScreenshotService { @@ -15,6 +17,8 @@ abstract class ScreenshotService public const THUMBS_FOLDER = 'thumbs/'; private const THUMB_WIDTH = 280; private const QUALITY = 50; + /** Lifetime of signed / presigned screenshot URLs */ + public const URL_TTL_MINUTES = 10; /** Get screenshot path by interval */ abstract public function getScreenshotPath(TimeInterval|int $interval): string; @@ -23,10 +27,8 @@ abstract public function getThumbPath(TimeInterval|int $interval): string; public function saveScreenshot($file, $timeInterval): void { - if (!Storage::exists(self::PARENT_FOLDER)) { - Storage::makeDirectory(self::PARENT_FOLDER); - } - + // $file is a local path or UploadedFile; the result goes to the default disk (local or S3). + // Storage::put() creates missing directories itself, no makeDirectory() round-trips on S3. $path = is_string($file) ? $file : $file->path(); $image = Image::make($path); @@ -38,11 +40,7 @@ public function saveScreenshot($file, $timeInterval): void public function createThumbnail(TimeInterval|int $timeInterval): void { - if (!Storage::exists(self::PARENT_FOLDER . self::THUMBS_FOLDER)) { - Storage::makeDirectory(self::PARENT_FOLDER . self::THUMBS_FOLDER); - } - - $image = Image::make(Storage::path($this->getScreenshotPath($timeInterval))); + $image = Image::make(Storage::get($this->getScreenshotPath($timeInterval))); $thumb = $image->resize(self::THUMB_WIDTH, null, fn(Constraint $constraint) => $constraint->aspectRatio()); @@ -55,6 +53,44 @@ public function destroyScreenshot(TimeInterval|int $interval): void Storage::delete($this->getThumbPath($interval)); } + /** + * Temporary URL of the screenshot (or its thumbnail) that browsers can load directly. + * + * Local disk: relative URL of a route protected by the "signed" middleware that streams the file. + * S3: presigned object URL. + */ + public function getTemporaryUrl(TimeInterval|int $interval, bool $thumbnail = false): string + { + $id = $interval instanceof TimeInterval ? $interval->id : $interval; + $expiresAt = now()->addMinutes(self::URL_TTL_MINUTES); + + if (StorageHelper::isLocal()) { + return URL::temporarySignedRoute( + $thumbnail ? 'intervals.screenshot.thumb' : 'intervals.screenshot.original', + $expiresAt, + ['interval' => $id], + absolute: false, + ); + } + + return Storage::temporaryUrl( + $thumbnail ? $this->getThumbPath($id) : $this->getScreenshotPath($id), + $expiresAt, + ); + } + + /** + * @return array{screenshot_url: string, thumbnail_url: string, expires_in: int} + */ + public function getTemporaryUrls(TimeInterval|int $interval): array + { + return [ + 'screenshot_url' => $this->getTemporaryUrl($interval), + 'thumbnail_url' => $this->getTemporaryUrl($interval, true), + 'expires_in' => self::URL_TTL_MINUTES * 60, + ]; + } + public static function getFullPath(): string { $fileSystemPath = config('filesystems.default'); diff --git a/app/Helpers/StorageCleaner.php b/app/Helpers/StorageCleaner.php index 0f1bbbda6..600649bc3 100644 --- a/app/Helpers/StorageCleaner.php +++ b/app/Helpers/StorageCleaner.php @@ -9,21 +9,45 @@ use Illuminate\Database\Eloquent\Builder; use Storage; +/** + * Screenshot rotation by free space of the local disk (PVC). + * + * Object storage (FILESYSTEM_DISK=s3) has no "free space": rotation is a no-op there, + * retention is expected to be handled by bucket lifecycle rules. + */ class StorageCleaner { + public static function isSupported(): bool + { + return StorageHelper::isLocal(); + } public static function getFreeSpace(): float { - return disk_free_space(Storage::path(ScreenshotService::PARENT_FOLDER)); + if (!self::isSupported()) { + return 0; + } + + $directory = Storage::path(ScreenshotService::PARENT_FOLDER); + + return disk_free_space(is_dir($directory) ? $directory : Storage::path('')); } public static function getUsedSpace(): float { + if (!self::isSupported()) { + return 0; + } + return config('cleaner.total_space') - self::getFreeSpace(); } public static function needThinning(): bool { + if (!self::isSupported()) { + return false; + } + return self::getUsedSpace() * 100 / config('cleaner.total_space') >= config('cleaner.threshold'); @@ -34,7 +58,10 @@ public static function needThinning(): bool */ public static function thin($force = false): void { - if ((!$force && !self::needThinning()) || Cache::store('octane')->get('thinning_now')) { + if (!self::isSupported() + || (!$force && !self::needThinning()) + || Cache::store('octane')->get('thinning_now') + ) { return; } @@ -91,6 +118,11 @@ public static function getAvailableScreenshots(): array */ public static function countAvailableScreenshots(): int { + // One existence check per interval is fine on a local disk, but would be a HEAD request each on S3. + if (!self::isSupported()) { + return 0; + } + $count = 0; $service = app()->make(ScreenshotService::class); diff --git a/app/Helpers/StorageHelper.php b/app/Helpers/StorageHelper.php new file mode 100644 index 000000000..7a7e274a8 --- /dev/null +++ b/app/Helpers/StorageHelper.php @@ -0,0 +1,19 @@ + config('cleaner.threshold'), 'need_thinning' => StorageCleaner::needThinning(), + // false on object storage (S3): no free space to measure, rotation is a no-op + 'rotation_supported' => StorageCleaner::isSupported(), 'screenshots_available' => StorageCleaner::countAvailableScreenshots(), 'thinning' => [ 'now' => Cache::store('octane')->get('thinning_now'), diff --git a/app/Http/Controllers/Api/AttachmentController.php b/app/Http/Controllers/Api/AttachmentController.php index 9eff2eaf7..c12afc890 100644 --- a/app/Http/Controllers/Api/AttachmentController.php +++ b/app/Http/Controllers/Api/AttachmentController.php @@ -48,11 +48,25 @@ public function tmpDownload(Request $request, Attachment $attachment): ?Streamed public function createTemporaryUrl(DownloadAttachmentRequest $request, Attachment $attachment): string { + // Only verified files are downloadable, on every disk (streamDownloadLogic() checks the same for the + // local route; a presigned S3 URL bypasses the app, so the check has to happen before issuing it) + if ($attachment->status !== AttachmentStatus::GOOD) { + abort(404); + } + + // query string values arrive as strings, Carbon 3 rejects them + $expiresAt = now()->addSeconds((int)($request->validated('seconds') ?? 3600)); + + // Object storage (S3): a presigned URL of the object itself, absolute + if ($presignedUrl = $this->attachmentService->getTemporaryUrl($attachment, $expiresAt)) { + return $presignedUrl; + } + // we do this because signature breaks if we use built in method for creating relative path // also cannot determine request scheme when ran inside docker $url = URL::temporarySignedRoute( 'attachment.temporary-download', - now()->addSeconds((int)($request->validated('seconds') ?? 3600)), + $expiresAt, $attachment ); $parsedUrl = parse_url($url); diff --git a/app/Http/Controllers/Api/IntervalController.php b/app/Http/Controllers/Api/IntervalController.php index b2782165f..49f6ad24d 100644 --- a/app/Http/Controllers/Api/IntervalController.php +++ b/app/Http/Controllers/Api/IntervalController.php @@ -4,6 +4,7 @@ use App\Contracts\ScreenshotService; use App\Enums\ScreenshotsState; +use App\Exceptions\Entities\AuthorizationException; use App\Http\Requests\Interval\BulkDestroyTimeIntervalRequest; use App\Http\Requests\Interval\BulkEditTimeIntervalRequest; use App\Http\Requests\Interval\CreateTimeIntervalRequest; @@ -28,6 +29,7 @@ use Exception; use Illuminate\Database\Eloquent\Model; use Illuminate\Http\JsonResponse; +use Illuminate\Http\Request; use Illuminate\Http\UploadedFile; use Illuminate\Support\Arr; use MessagePack\MessagePack; @@ -36,7 +38,7 @@ use Spatie\TemporaryDirectory\TemporaryDirectory; use Storage; use Str; -use Symfony\Component\HttpFoundation\BinaryFileResponse; +use Symfony\Component\HttpFoundation\StreamedResponse; use Throwable; use Validator; use ZipArchive; @@ -86,6 +88,14 @@ public function index(ListIntervalRequest $request): JsonResponse return $filters; }); + Filter::listen(Filter::getActionFilterName(), static function ($items) { + $items->each( + static fn (TimeInterval $interval) => $interval->append(TimeInterval::SCREENSHOT_URL_ATTRIBUTES) + ); + + return $items; + }); + return $this->_index($request); } @@ -123,6 +133,11 @@ public function index(ListIntervalRequest $request): JsonResponse */ public function show(ShowIntervalRequest $request): JsonResponse { + Filter::listen( + Filter::getActionFilterName(), + static fn (?TimeInterval $interval) => $interval?->append(TimeInterval::SCREENSHOT_URL_ATTRIBUTES), + ); + return $this->_show($request); } @@ -450,7 +465,6 @@ public function trackApp(TrackAppRequest $request): JsonResponse * @apiSuccess {String} created_at Creation timestamp * @apiSuccess {String} updated_at Last update timestamp * @apiSuccess {Boolean} is_manual Indicates whether the time was logged manually (true) or automatically - * @apiSuccess {Integer} has_screenshot Indicates if there is a screenshot for this interval * * @apiSuccessExample {json} Response Example * HTTP/1.1 200 OK @@ -462,8 +476,7 @@ public function trackApp(TrackAppRequest $request): JsonResponse * "end_at": "2024-08-16T18:10:00.000000Z", * "created_at": "2024-08-16T20:07:14.000000Z", * "updated_at": "2024-08-16T20:07:14.000000Z", - * "is_manual": true, - * "has_screenshot": true + * "is_manual": true * } * * @apiUse 400Error @@ -488,7 +501,8 @@ static function (array $requestData) { $screenshotService = $this->screenshotService; if ($request->hasFile('screenshot') && optional($request->file('screenshot'))->isValid()) { - $path = $request->file('screenshot')->store('tmp'); + // The upload is only a local temporary copy; the encoded screenshot goes to the default disk. + $path = $request->file('screenshot')->store('tmp', 'local'); CatEvent::listen( Filter::getAfterActionEventName(), @@ -499,8 +513,8 @@ static function (TimeInterval $interval) use ($path, $screenshotService) { if ($mustCapture || ($optionalCapture && $interval->user->screenshots_state === ScreenshotsState::REQUIRED) ) { - $screenshotService->saveScreenshot(Storage::path($path), $interval); - dispatch(static fn() => Storage::delete($path))->delay(now()->addMinute()); + $screenshotService->saveScreenshot(Storage::disk('local')->path($path), $interval); + dispatch(static fn() => Storage::disk('local')->delete($path))->delay(now()->addMinute()); } } ); @@ -518,28 +532,96 @@ static function ($data) { return $this->_create($request); } - public function showScreenshot(ScreenshotRequest $request, TimeInterval $interval): BinaryFileResponse + /** + * @api {get} /time-intervals/:id/screenshot Screenshot + * @apiDescription Stream the screenshot of an interval. Not authenticated: access is granted by the + * temporary signature (10 min) of the URL from `screenshot_url` or `screenshot-urls`. + * Used only with the local disk; on S3 the URLs point to the bucket directly. + * + * @apiVersion 4.0.0 + * @apiName Screenshot + * @apiGroup Time Interval + * + * @apiParam {Integer} expires Signature expiration timestamp + * @apiParam {String} signature URL signature + * + * @apiSuccess {File} screenshot JPEG image + * @apiError (403) InvalidSignature Missing, invalid or expired signature + * @apiUse ItemNotFoundError + */ + public function showScreenshot(Request $request, int $interval): StreamedResponse { - $path = $this->screenshotService->getScreenshotPath($interval); - if (!Storage::exists($path)) { - abort(404); - } - - $fullPath = Storage::path($path); + return $this->streamScreenshot($request, $this->screenshotService->getScreenshotPath($interval)); + } - return response()->file($fullPath); + /** + * @api {get} /time-intervals/:id/thumb Thumbnail + * @apiDescription Stream the screenshot thumbnail of an interval, see Screenshot. + * + * @apiVersion 4.0.0 + * @apiName Thumbnail + * @apiGroup Time Interval + * + * @apiParam {Integer} expires Signature expiration timestamp + * @apiParam {String} signature URL signature + * + * @apiSuccess {File} thumbnail JPEG image + * @apiError (403) InvalidSignature Missing, invalid or expired signature + * @apiUse ItemNotFoundError + */ + public function showThumbnail(Request $request, int $interval): StreamedResponse + { + return $this->streamScreenshot($request, $this->screenshotService->getThumbPath($interval)); } - public function showThumbnail(ScreenshotRequest $request, TimeInterval $interval): BinaryFileResponse + /** + * @throws Throwable + */ + private function streamScreenshot(Request $request, string $path): StreamedResponse { - $path = $this->screenshotService->getThumbPath($interval); - if (!Storage::exists($path)) { - abort(404); - } + // Checked here rather than by the "signed" middleware: its InvalidSignatureException renders as 500 + throw_unless( + $request->hasValidSignature(absolute: false), + new AuthorizationException(AuthorizationException::ERROR_TYPE_FORBIDDEN) + ); + abort_unless(Storage::exists($path), 404); - $fullPath = Storage::path($path); + return Storage::response($path, headers: [ + 'Content-Type' => 'image/jpeg', + 'Cache-Control' => 'private, max-age=' . ScreenshotService::URL_TTL_MINUTES * 60, + ]); + } - return response()->file($fullPath); + /** + * @api {get} /time-intervals/:id/screenshot-urls Screenshot URLs + * @apiDescription Fresh temporary URLs of the interval screenshot and thumbnail + * (for pages open longer than the URL lifetime and for report payloads without URLs) + * + * @apiVersion 4.0.0 + * @apiName ScreenshotUrls + * @apiGroup Time Interval + * + * @apiUse AuthHeader + * + * @apiSuccess {String} screenshot_url Temporary URL of the screenshot + * @apiSuccess {String} thumbnail_url Temporary URL of the thumbnail + * @apiSuccess {Integer} expires_in URL lifetime in seconds + * + * @apiSuccessExample {json} Response Example + * HTTP/1.1 200 OK + * { + * "screenshot_url": "/api/time-intervals/1/screenshot?expires=1698316277&signature=...", + * "thumbnail_url": "/api/time-intervals/1/thumb?expires=1698316277&signature=...", + * "expires_in": 600 + * } + * + * @apiUse UnauthorizedError + * @apiUse ForbiddenError + * @apiUse ItemNotFoundError + */ + public function screenshotUrls(ScreenshotRequest $request, TimeInterval $interval): JsonResponse + { + return responder()->success($this->screenshotService->getTemporaryUrls($interval))->respond(); } public function putScreenshot(PutScreenshotRequest $request, TimeInterval $interval): JsonResponse @@ -761,7 +843,7 @@ public function uploadOfflineScreenshots(UploadOfflineScreenshotsRequest $reques } try { dispatch(static function () use ($screenshotService, $interval, $screenshotPath) { - $screenshotService->saveScreenshot(Storage::path($screenshotPath), $interval); + $screenshotService->saveScreenshot(Storage::disk('local')->path($screenshotPath), $interval); $interval->screenshot_id = null; $interval->save(); }); diff --git a/app/Http/Controllers/Api/ItemController.php b/app/Http/Controllers/Api/ItemController.php index d1913716b..aec058904 100644 --- a/app/Http/Controllers/Api/ItemController.php +++ b/app/Http/Controllers/Api/ItemController.php @@ -289,7 +289,8 @@ abstract class ItemController extends Controller * @apiSuccess {Integer} keyboard_fill Keyboard activity fill percentage * @apiSuccess {Integer} location Additional location information, if available * @apiSuccess {Integer} screenshot_id The ID of the screenshot associated with this interval - * @apiSuccess {Integer} has_screenshot Indicates if there is a screenshot for this interval + * @apiSuccess {String} screenshot_url Temporary (10 min) URL of the screenshot, list/show only + * @apiSuccess {String} thumbnail_url Temporary (10 min) URL of the screenshot thumbnail, list/show only * @apiSuccessExample {json} Response Example * HTTP/1.1 200 OK * { @@ -308,7 +309,8 @@ abstract class ItemController extends Controller * "keyboard_fill": 13, * "location": null, * "screenshot_id": null, - * "has_screenshot": true + * "screenshot_url": "/api/time-intervals/1/screenshot?expires=1698316277&signature=...", + * "thumbnail_url": "/api/time-intervals/1/thumb?expires=1698316277&signature=..." * },... * } */ diff --git a/app/Http/Requests/Attachment/DownloadAttachmentRequest.php b/app/Http/Requests/Attachment/DownloadAttachmentRequest.php index b25dec883..cc47110f5 100644 --- a/app/Http/Requests/Attachment/DownloadAttachmentRequest.php +++ b/app/Http/Requests/Attachment/DownloadAttachmentRequest.php @@ -2,6 +2,7 @@ namespace App\Http\Requests\Attachment; +use App\Contracts\AttachmentService; use App\Http\Requests\AuthorizesAfterValidation; use App\Http\Requests\CattrFormRequest; @@ -17,7 +18,8 @@ public function authorizeValidated(): bool public function _rules(): array { return [ - 'seconds' => 'sometimes|int' + // 7 days at most on every disk (the S3 presigning limit) + 'seconds' => 'sometimes|int|min:1|max:' . AttachmentService::MAX_TEMPORARY_URL_SECONDS, ]; } } diff --git a/app/Jobs/GenerateAndSendReport.php b/app/Jobs/GenerateAndSendReport.php index fd9e7e674..16f5da634 100644 --- a/app/Jobs/GenerateAndSendReport.php +++ b/app/Jobs/GenerateAndSendReport.php @@ -3,6 +3,7 @@ namespace App\Jobs; use App\Contracts\AppReport; +use App\Helpers\StorageHelper; use App\Models\User; use App\Notifications\ReportGenerated; use Illuminate\Bus\Queueable; @@ -15,6 +16,7 @@ use Illuminate\Validation\ValidationException; use Maatwebsite\Excel\Excel; use Storage; +use Symfony\Component\HttpFoundation\HeaderUtils; use Throwable; class GenerateAndSendReport implements ShouldQueue, ShouldBeUnique @@ -24,11 +26,30 @@ class GenerateAndSendReport implements ShouldQueue, ShouldBeUnique public int $uniqueFor = 60; private string $dir; - private const STORAGE_DRIVE = 'public'; + /** "reports" disk: storage/app/public (served at /storage) or S3, see REPORTS_DISK */ + private const STORAGE_DRIVE = 'reports'; + /** Lifetime of the presigned download URL when reports are on S3 */ + public const URL_TTL_MINUTES = 10; public function getPublicPath(): string { - return Storage::drive(self::STORAGE_DRIVE)->url($this->getReportPath()); + if (StorageHelper::isLocal(self::STORAGE_DRIVE)) { + return Storage::drive(self::STORAGE_DRIVE)->url($this->getReportPath()); + } + + $fileName = basename($this->getReportPath()); + + return Storage::drive(self::STORAGE_DRIVE)->temporaryUrl( + $this->getReportPath(), + now()->addMinutes(self::URL_TTL_MINUTES), + [ + 'ResponseContentDisposition' => HeaderUtils::makeDisposition( + HeaderUtils::DISPOSITION_ATTACHMENT, + $fileName, + str_replace(['%', '/', '\\'], '_', Str::ascii($fileName)) ?: 'report', + ), + ], + ); } /** @@ -49,8 +70,6 @@ public function __construct( */ public function handle(): void { - Storage::drive(self::STORAGE_DRIVE)->makeDirectory("reports/$this->dir"); - $fileName = $this->getReportPath(); $this->report->store($fileName, self::STORAGE_DRIVE, $this->type === 'pdf' ? Excel::MPDF : Str::ucfirst($this->type)); diff --git a/app/Models/TimeInterval.php b/app/Models/TimeInterval.php index 8b0860cc2..d728ecf7b 100644 --- a/app/Models/TimeInterval.php +++ b/app/Models/TimeInterval.php @@ -17,7 +17,6 @@ use Illuminate\Database\Query\Builder as QueryBuilder; use Illuminate\Notifications\Notifiable; use Illuminate\Support\Carbon; -use Storage; use MatanYadaev\EloquentSpatial\SpatialBuilder as Builder; use MatanYadaev\EloquentSpatial\Objects\Point; use MatanYadaev\EloquentSpatial\Traits\HasSpatial; @@ -41,7 +40,8 @@ * @property array|null $location * @property-read Collection|TrackedApplication[] $apps * @property-read int|null $apps_count - * @property-read bool $has_screenshot + * @property-read string $screenshot_url Signed / presigned URL, short-lived; appended only by the API + * @property-read string $thumbnail_url Signed / presigned URL, short-lived; appended only by the API * @property-read Collection|Property[] $properties * @property-read int|null $properties_count * @property-read Task $task @@ -141,8 +141,6 @@ class TimeInterval extends Model 'deleted_at', ]; - protected $appends = ['has_screenshot']; - /** * Override parent boot and Call deleting event * @@ -170,12 +168,20 @@ public function properties(): MorphMany return $this->morphMany(Property::class, 'entity'); } - public function hasScreenshot(): Attribute + /** Attributes the API appends to intervals it returns (see IntervalController) */ + public const SCREENSHOT_URL_ATTRIBUTES = ['screenshot_url', 'thumbnail_url']; + + public function screenshotUrl(): Attribute { - return Attribute::make( - get: static fn ($value) => !$value || Storage::exists( - app(ScreenshotService::class)->getScreenshotPath($value['id']) - ) + return Attribute::get( + fn () => app(ScreenshotService::class)->getTemporaryUrl($this->id) + )->shouldCache(); + } + + public function thumbnailUrl(): Attribute + { + return Attribute::get( + fn () => app(ScreenshotService::class)->getTemporaryUrl($this->id, true) )->shouldCache(); } diff --git a/app/Notifications/ReportGenerated.php b/app/Notifications/ReportGenerated.php index 8d525c9d5..77b153b25 100644 --- a/app/Notifications/ReportGenerated.php +++ b/app/Notifications/ReportGenerated.php @@ -6,12 +6,19 @@ use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; -use Storage; +use Illuminate\Support\Facades\Storage; +use RuntimeException; +use Throwable; class ReportGenerated extends Notification implements ShouldQueue { use Queueable; + /** + * Attachment buffer: up to 1 MiB in memory, the rest spills to a temporary file. + */ + private const BUFFER = 'php://temp/maxmemory:1048576'; + public function __construct(private $fileName, private $drive) { } @@ -25,7 +32,68 @@ public function toMail($notifiable): MailMessage { return (new MailMessage) ->line('In attachment see report you have requested.') - ->attach(Storage::drive($this->drive)->path($this->fileName)); + ->attachData( + $this->bufferReport(), + basename($this->fileName), + ['mime' => Storage::drive($this->drive)->mimeType($this->fileName) ?: null], + ); + } + + /** + * Copy the report into a seekable temporary stream instead of loading it into a string + * (Attachment::fromStorageDisk() calls Storage::get()). Symfony Mime accepts a resource as the + * attachment body and encodes it chunk by chunk, so the report is never held in memory as a whole. + * The copy also makes the body seekable (an S3 read stream is not), so the message can be rendered again. + * + * @return resource + */ + private function bufferReport() + { + $source = Storage::drive($this->drive)->readStream($this->fileName); + if (!is_resource($source)) { + throw new RuntimeException("Unable to read report $this->fileName from the $this->drive disk"); + } + + $buffer = false; + try { + $buffer = $this->openBuffer(); + if ($buffer === false) { + throw new RuntimeException("Unable to open a buffer for report $this->fileName"); + } + + // fail the job rather than mail a truncated report + $copied = stream_copy_to_stream($source, $buffer); + $expected = Storage::drive($this->drive)->size($this->fileName); + if ($copied !== $expected) { + throw new RuntimeException(sprintf( + 'Report %s: copied %s of %d bytes from the %s disk', + $this->fileName, + var_export($copied, true), + $expected, + $this->drive, + )); + } + + rewind($buffer); + } catch (Throwable $e) { + if (is_resource($buffer)) { + fclose($buffer); + } + + throw $e; + } finally { + fclose($source); + } + + return $buffer; + } + + /** + * @return resource|false + */ + protected function openBuffer() + { + return fopen(self::BUFFER, 'w+b'); } public function toArray($notifiable): array diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 7bfe1a463..c8b87af16 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -8,6 +8,9 @@ use App\Models\User; use App\Observers\TimeIntervalObserver; use App\Observers\UserObserver; +use Illuminate\Cache\RateLimiting\Limit; +use Illuminate\Http\Request; +use Illuminate\Support\Facades\RateLimiter; use Illuminate\Support\ServiceProvider; use Laravel\Tinker\TinkerServiceProvider; use Settings; @@ -23,6 +26,12 @@ public function boot(): void User::observe(UserObserver::class); TimeInterval::observe(TimeIntervalObserver::class); config(['app.timezone' => Settings::scope('core')->get('timezone', date_default_timezone_get())]); + + // Public signed screenshot routes: no user to key by, so per IP + RateLimiter::for( + 'screenshots', + static fn (Request $request) => Limit::perMinute(config('app.screenshots_rate_limit'))->by($request->ip()) + ); } /** diff --git a/app/Services/AttachmentService.php b/app/Services/AttachmentService.php index dfb015523..8071a1e01 100644 --- a/app/Services/AttachmentService.php +++ b/app/Services/AttachmentService.php @@ -4,14 +4,19 @@ use App\Contracts\AttachmentAble; use App\Enums\AttachmentStatus; +use App\Helpers\StorageHelper; use App\Jobs\VerifyAttachmentHash; use App\Models\Attachment; use App\Models\Project; use App\Models\Task; use App\Models\TaskComment; +use DateTimeInterface; use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Str; +use RuntimeException; use Storage; +use Symfony\Component\HttpFoundation\HeaderUtils; class AttachmentService implements \App\Contracts\AttachmentService { @@ -101,11 +106,15 @@ public function handleProjectDeletion(Project $project): void { Attachment::whereProjectId($project->id)->delete(); - if ($this->storage->directoryExists($this->getProjectPath($project)) && !$this->storage->deleteDirectory($this->getProjectPath($project))) { - \Log::error("Unable to delete project's attachments directory", [ - 'project_id' => $project->id, - 'path' => $this->getProjectPath($project) - ]); + // the project's quarantined (BAD) files go too: their records are gone, nothing refers to them anymore + $projectPath = $this->getProjectPath($project); + foreach ([$projectPath, $this->getQuarantinePath($projectPath)] as $path) { + if ($this->storage->directoryExists($path) && !$this->storage->deleteDirectory($path)) { + \Log::error("Unable to delete project's attachments directory", [ + 'project_id' => $project->id, + 'path' => $path + ]); + } } } @@ -153,16 +162,47 @@ public function getPath(Attachment $attachment): string return match ($attachment->status) { AttachmentStatus::NOT_ATTACHED => "users/{$attachment->user_id}/{$attachment->id}", AttachmentStatus::PROCESSING, - AttachmentStatus::GOOD, - AttachmentStatus::BAD => "projects/{$attachment->project_id}/{$attachment->id}", + AttachmentStatus::GOOD => $this->getProjectFilePath($attachment), + AttachmentStatus::BAD => $this->getQuarantinePath($this->getProjectFilePath($attachment)), }; } + private function getProjectFilePath(Attachment $attachment): string + { + return "projects/{$attachment->project_id}/{$attachment->id}"; + } + + private function getQuarantinePath(string $path): string + { + return $this::QUARANTINE_DIR . "/$path"; + } + public function getFullPath(Attachment $attachment): string { return $this->storage->path($this->getPath($attachment)); } + public function getTemporaryUrl(Attachment $attachment, DateTimeInterface $expiresAt): ?string + { + if (StorageHelper::isLocal($this::DISK)) { + return null; + } + + $options = [ + // Same download headers as AttachmentController::streamDownloadLogic() sends for the local disk + 'ResponseContentDisposition' => HeaderUtils::makeDisposition( + HeaderUtils::DISPOSITION_ATTACHMENT, + $attachment->original_name, + str_replace(['%', '/', '\\'], '_', Str::ascii($attachment->original_name)) ?: 'attachment', + ), + ]; + if ($attachment->mime_type !== '') { + $options['ResponseContentType'] = $attachment->mime_type; + } + + return $this->storage->temporaryUrl($this->getPath($attachment), $expiresAt, $options); + } + public function readStream(Attachment $attachment) { return $this->storage->readStream($this->getPath($attachment)); @@ -195,9 +235,33 @@ public function verifyHash(Attachment $attachment): void $attachment->status = AttachmentStatus::GOOD; } elseif ($attachment->status === AttachmentStatus::GOOD && $attachment->hash !== $this->getHashSum($attachment) ) { + $this->quarantine($attachment); $attachment->status = AttachmentStatus::BAD; } + // BAD is final: the file stays quarantined and is never verified (and turned GOOD) again $attachment->save(); } + + /** + * Move the file of an attachment that is about to become BAD from its public key to the quarantine key. + * Throws while the file stays at the public key, so the attachment is not marked BAD while presigned URLs + * issued for it still work: the verification job fails and is retried. + */ + private function quarantine(Attachment $attachment): void + { + $from = $this->getProjectFilePath($attachment); + $to = $this->getQuarantinePath($from); + + // Nothing at the public key: the file is missing, or a previous attempt has moved it but failed to save + // the status. Either way nothing is left to serve, the attachment can become BAD. + if (!$this->storage->exists($from)) { + return; + } + + // S3 has no rename: move() is copy + delete, check the public key is really gone + if (!$this->storage->move($from, $to) || $this->storage->exists($from)) { + throw new RuntimeException("Unable to quarantine the file of attachment $attachment->id ($from -> $to)"); + } + } } diff --git a/bootstrap/app.php b/bootstrap/app.php index 709d9a546..5ad0ebe59 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -62,6 +62,11 @@ 'ability' => CheckForAnyAbility::class, ]); + // Octane is only reached through the bundled Caddy (runtime/caddy/Caddyfile, upstream 127.0.0.1:8090), + // which sets X-Forwarded-* and passes on incoming ones only from its own trusted_proxies. Trust exactly + // that hop, so request()->ip() (rate limiters, recaptcha, desktop keys) is the client, not 127.0.0.1. + $middleware->trustProxies(at: ['127.0.0.1']); + $middleware->trimStrings(except: [ 'password', 'password_confirmation', diff --git a/composer.json b/composer.json index f2e0e24a1..90220fc26 100644 --- a/composer.json +++ b/composer.json @@ -29,6 +29,7 @@ "laravel/octane": "^2.2", "laravel/reverb": "^1.0", "laravel/sanctum": "^4.0", + "league/flysystem-aws-s3-v3": "^3.0", "maatwebsite/excel": "^3.1", "matanyadaev/laravel-eloquent-spatial": "^4.8", "mpdf/mpdf": "^8.0", diff --git a/composer.lock b/composer.lock index 15d049767..12d2c0dcf 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,159 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "6ac5d0f36506e500119b5bb642c9a058", + "content-hash": "13ca53e9d9bd13a0313293294940c9fc", "packages": [ + { + "name": "aws/aws-crt-php", + "version": "v1.2.7", + "source": { + "type": "git", + "url": "https://github.com/awslabs/aws-crt-php.git", + "reference": "d71d9906c7bb63a28295447ba12e74723bd3730e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/awslabs/aws-crt-php/zipball/d71d9906c7bb63a28295447ba12e74723bd3730e", + "reference": "d71d9906c7bb63a28295447ba12e74723bd3730e", + "shasum": "" + }, + "require": { + "php": ">=5.5" + }, + "require-dev": { + "phpunit/phpunit": "^4.8.35||^5.6.3||^9.5", + "yoast/phpunit-polyfills": "^1.0" + }, + "suggest": { + "ext-awscrt": "Make sure you install awscrt native extension to use any of the functionality." + }, + "type": "library", + "autoload": { + "classmap": [ + "src/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Apache-2.0" + ], + "authors": [ + { + "name": "AWS SDK Common Runtime Team", + "email": "aws-sdk-common-runtime@amazon.com" + } + ], + "description": "AWS Common Runtime for PHP", + "homepage": "https://github.com/awslabs/aws-crt-php", + "keywords": [ + "amazon", + "aws", + "crt", + "sdk" + ], + "support": { + "issues": "https://github.com/awslabs/aws-crt-php/issues", + "source": "https://github.com/awslabs/aws-crt-php/tree/v1.2.7" + }, + "time": "2024-10-18T22:15:13+00:00" + }, + { + "name": "aws/aws-sdk-php", + "version": "3.402.0", + "source": { + "type": "git", + "url": "https://github.com/aws/aws-sdk-php.git", + "reference": "661f6a54f2a63ac5137390c291535c74cfe6019e" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/aws/aws-sdk-php/zipball/661f6a54f2a63ac5137390c291535c74cfe6019e", + "reference": "661f6a54f2a63ac5137390c291535c74cfe6019e", + "shasum": "" + }, + "require": { + "aws/aws-crt-php": "^1.2.3", + "ext-json": "*", + "ext-pcre": "*", + "ext-simplexml": "*", + "guzzlehttp/guzzle": "^7.8.2 || ^8.0", + "guzzlehttp/promises": "^2.0.3 || ^3.0", + "guzzlehttp/psr7": "^2.6.3 || ^3.0", + "mtdowling/jmespath.php": "^2.9.1", + "php": ">=8.2", + "psr/http-message": "^1.0 || ^2.0", + "symfony/filesystem": "^v5.4.45 || ^v6.4.3 || ^v7.1.0 || ^v8.0.0" + }, + "require-dev": { + "andrewsville/php-token-reflection": "^1.4", + "aws/aws-php-sns-message-validator": "~1.0", + "behat/behat": "~3.0", + "composer/composer": "^2.7.8", + "dms/phpunit-arraysubset-asserts": "^v0.5.0", + "doctrine/cache": "~1.4", + "ext-dom": "*", + "ext-openssl": "*", + "ext-sockets": "*", + "phpunit/phpunit": "^10.0", + "psr/cache": "^2.0 || ^3.0", + "psr/simple-cache": "^2.0 || ^3.0", + "sebastian/comparator": "^1.2.3 || ^4.0 || ^5.0", + "yoast/phpunit-polyfills": "^2.0" + }, + "suggest": { + "aws/aws-php-sns-message-validator": "To validate incoming SNS notifications", + "doctrine/cache": "To use the DoctrineCacheAdapter", + "ext-curl": "To send requests using cURL", + "ext-openssl": "Allows working with CloudFront private distributions and verifying received SNS messages", + "ext-pcntl": "To use client-side monitoring", + "ext-sockets": "To use client-side monitoring" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.0-dev" + } + }, + "autoload": { + "files": [ + "src/functions.php" + ], + "psr-4": { + "Aws\\": "src/" + }, + "exclude-from-classmap": [ + "src/data/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "Apache-2.0" + ], + "authors": [ + { + "name": "Amazon Web Services", + "homepage": "https://aws.amazon.com" + } + ], + "description": "AWS SDK for PHP - Use Amazon Web Services in your PHP project", + "homepage": "https://aws.amazon.com/sdk-for-php", + "keywords": [ + "amazon", + "aws", + "cloud", + "dynamodb", + "ec2", + "glacier", + "s3", + "sdk" + ], + "support": { + "forum": "https://github.com/aws/aws-sdk-php/discussions", + "issues": "https://github.com/aws/aws-sdk-php/issues", + "source": "https://github.com/aws/aws-sdk-php/tree/3.402.0" + }, + "time": "2026-10-22T18:09:05+00:00" + }, { "name": "brick/geo", "version": "0.13.1", @@ -2850,6 +3001,61 @@ }, "time": "2026-09-02T08:00:27+00:00" }, + { + "name": "league/flysystem-aws-s3-v3", + "version": "3.35.3", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/flysystem-aws-s3-v3.git", + "reference": "b03780cb97585ee7e48977f40ed599b33b751634" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/flysystem-aws-s3-v3/zipball/b03780cb97585ee7e48977f40ed599b33b751634", + "reference": "b03780cb97585ee7e48977f40ed599b33b751634", + "shasum": "" + }, + "require": { + "aws/aws-sdk-php": "^3.371.5", + "league/flysystem": "^3.10.0", + "league/mime-type-detection": "^1.0.0", + "php": "^8.0.2" + }, + "conflict": { + "guzzlehttp/guzzle": "<7.0", + "guzzlehttp/ringphp": "<1.1.1" + }, + "type": "library", + "autoload": { + "psr-4": { + "League\\Flysystem\\AwsS3V3\\": "" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Frank de Jonge", + "email": "info@frankdejonge.nl" + } + ], + "description": "AWS S3 filesystem adapter for Flysystem.", + "keywords": [ + "Flysystem", + "aws", + "file", + "files", + "filesystem", + "s3", + "storage" + ], + "support": { + "source": "https://github.com/thephpleague/flysystem-aws-s3-v3/tree/3.35.3" + }, + "time": "2026-08-08T16:19:23+00:00" + }, { "name": "league/flysystem-local", "version": "3.35.3", @@ -3817,6 +4023,72 @@ }, "time": "2023-05-03T06:19:36+00:00" }, + { + "name": "mtdowling/jmespath.php", + "version": "2.9.2", + "source": { + "type": "git", + "url": "https://github.com/jmespath/jmespath.php.git", + "reference": "2157c5e50e813ec6a96c1eed3be7f64a20fb32a8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/jmespath/jmespath.php/zipball/2157c5e50e813ec6a96c1eed3be7f64a20fb32a8", + "reference": "2157c5e50e813ec6a96c1eed3be7f64a20fb32a8", + "shasum": "" + }, + "require": { + "php": "^7.2.5 || ^8.0", + "symfony/polyfill-mbstring": "^1.17" + }, + "require-dev": { + "composer/xdebug-handler": "^3.0.3", + "phpunit/phpunit": "^8.5.52" + }, + "bin": [ + "bin/jp.php" + ], + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.9-dev" + } + }, + "autoload": { + "files": [ + "src/JmesPath.php" + ], + "psr-4": { + "JmesPath\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Graham Campbell", + "email": "hello@gjcampbell.co.uk", + "homepage": "https://github.com/GrahamCampbell" + }, + { + "name": "Michael Dowling", + "email": "mtdowling@gmail.com", + "homepage": "https://github.com/mtdowling" + } + ], + "description": "Declaratively specify how to extract elements from a JSON document", + "keywords": [ + "json", + "jsonpath" + ], + "support": { + "issues": "https://github.com/jmespath/jmespath.php/issues", + "source": "https://github.com/jmespath/jmespath.php/tree/2.9.2" + }, + "time": "2026-07-06T18:56:19+00:00" + }, { "name": "myclabs/deep-copy", "version": "1.14.0", @@ -6998,6 +7270,76 @@ ], "time": "2026-06-05T06:23:12+00:00" }, + { + "name": "symfony/filesystem", + "version": "v7.4.21", + "source": { + "type": "git", + "url": "https://github.com/symfony/filesystem.git", + "reference": "fd0e3462be6d32a0d130e8423e4940a7688162d8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/symfony/filesystem/zipball/fd0e3462be6d32a0d130e8423e4940a7688162d8", + "reference": "fd0e3462be6d32a0d130e8423e4940a7688162d8", + "shasum": "" + }, + "require": { + "php": ">=8.2", + "symfony/polyfill-ctype": "~1.8", + "symfony/polyfill-mbstring": "~1.8" + }, + "require-dev": { + "symfony/process": "^6.4|^7.0|^8.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Symfony\\Component\\Filesystem\\": "" + }, + "exclude-from-classmap": [ + "/Tests/" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Fabien Potencier", + "email": "fabien@symfony.com" + }, + { + "name": "Symfony Community", + "homepage": "https://symfony.com/contributors" + } + ], + "description": "Provides basic utilities for the filesystem", + "homepage": "https://symfony.com", + "support": { + "source": "https://github.com/symfony/filesystem/tree/v7.4.21" + }, + "funding": [ + { + "url": "https://symfony.com/sponsor", + "type": "custom" + }, + { + "url": "https://github.com/fabpot", + "type": "github" + }, + { + "url": "https://github.com/nicolas-grekas", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/symfony/symfony", + "type": "tidelift" + } + ], + "time": "2026-10-06T20:05:19+00:00" + }, { "name": "symfony/finder", "version": "v7.4.20", diff --git a/config/app.php b/config/app.php index 0fefee9d8..717e6a8f4 100644 --- a/config/app.php +++ b/config/app.php @@ -35,6 +35,9 @@ 'store' => env('APP_MAINTENANCE_STORE', 'database'), // 'store' => 'redis', ], + // Requests per minute and IP to the public signed screenshot routes (time-intervals/{id}/screenshot|thumb). + // A timeline day of a user with 1-minute screenshots holds up to ~600 thumbnails (10 h of work). + 'screenshots_rate_limit' => (int) env('SCREENSHOTS_RATE_LIMIT', 600), 'user_activity' => [ 'online_status_time' => 60, 'heartbeat_period' => 30, diff --git a/config/filesystems.php b/config/filesystems.php index 27341aabe..6fc4ffd3d 100644 --- a/config/filesystems.php +++ b/config/filesystems.php @@ -1,4 +1,30 @@ [ + 'driver' => 's3', + 'key' => env('AWS_ACCESS_KEY_ID'), + 'secret' => env('AWS_SECRET_ACCESS_KEY'), + 'region' => env('AWS_DEFAULT_REGION', 'us-east-1'), + 'bucket' => env('AWS_BUCKET'), + 'url' => env('AWS_URL'), + // Presigned links are signed for this host (SigV4 signs "Host"), so prefer an endpoint that both the + // app and browsers reach under the same name, e.g. the public MinIO URL. + 'endpoint' => env('AWS_ENDPOINT'), + // Only rewrites scheme, host and port of an already signed link (path ignored). Use it only for a CDN / + // proxy that forwards requests to the storage with the original AWS_ENDPOINT Host, or signatures fail. + 'temporary_url' => env('AWS_TEMPORARY_URL'), + // MinIO and most self-hosted S3 implementations need path-style URLs. + 'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false), + 'root' => trim(trim((string) env('AWS_ROOT', ''), '/') . '/' . $root, '/'), + 'visibility' => 'private', + 'throw' => false, + 'report' => false, +]; + return [ /* @@ -12,6 +38,7 @@ | */ + // Screenshots and thumbnails live on the default disk: "local" (PVC / storage/app) or "s3". 'default' => env('FILESYSTEM_DISK', 'local'), /* @@ -45,26 +72,25 @@ 'report' => false, ], - 'attachments' => [ + // ATTACHMENTS_DISK: "local" (storage/app/attachments) or "s3" (/attachments). + 'attachments' => (env('ATTACHMENTS_DISK', 'local') === 's3' ? $s3Disk('attachments') : [ 'driver' => 'local', 'root' => storage_path('app/attachments'), 'throw' => false, - 'checksum_algo' => 'sha512/256' - ], + ]) + ['checksum_algo' => 'sha512/256'], - 's3' => [ - 'driver' => 's3', - 'key' => env('AWS_ACCESS_KEY_ID'), - 'secret' => env('AWS_SECRET_ACCESS_KEY'), - 'region' => env('AWS_DEFAULT_REGION'), - 'bucket' => env('AWS_BUCKET'), - 'url' => env('AWS_URL'), - 'endpoint' => env('AWS_ENDPOINT'), - 'use_path_style_endpoint' => env('AWS_USE_PATH_STYLE_ENDPOINT', false), + // REPORTS_DISK: "local" (storage/app/public/reports, served at /storage) or "s3" (/reports). + 'reports' => env('REPORTS_DISK', 'local') === 's3' ? $s3Disk() : [ + 'driver' => 'local', + 'root' => storage_path('app/public'), + 'url' => '/storage', + 'visibility' => 'public', 'throw' => false, 'report' => false, ], + 's3' => $s3Disk(), + ], /* diff --git a/docs/developer/en/backend/README.md b/docs/developer/en/backend/README.md index d0fda39a9..e83fdb650 100644 --- a/docs/developer/en/backend/README.md +++ b/docs/developer/en/backend/README.md @@ -4,6 +4,7 @@ These documents describe implementation details of the legacy PHP backend. The J - [Access policy and roles](access-policy.md) - [Events and filters](events-and-filters.md) +- [File storage: local disk or S3](storage.md) - [Building an integration module (Jira example)](jira-integration.md) - [HTTP API reference](https://docs.cattr.app/api/) diff --git a/docs/developer/en/backend/storage.md b/docs/developer/en/backend/storage.md new file mode 100644 index 000000000..cb4751172 --- /dev/null +++ b/docs/developer/en/backend/storage.md @@ -0,0 +1,44 @@ +# File storage: local disk or S3 + +Cattr keeps three kinds of files. Each of them lives either on the local disk (`storage/app`, a volume / PVC) or in an S3-compatible bucket (AWS S3, MinIO, ...). + +| Files | Disk | Selected by | Local location | S3 key prefix | +|---|---|---|---|---| +| Screenshots and thumbnails | default disk | `FILESYSTEM_DISK=local\|s3` | `storage/app/screenshots/` | `/screenshots/` | +| Task attachments | `attachments` | `ATTACHMENTS_DISK=local\|s3` | `storage/app/attachments/` | `/attachments/` | +| Generated reports | `reports` | `REPORTS_DISK=local\|s3` | `storage/app/public/reports/` (served at `/storage`) | `/reports/` | + +Temporary files (uploads being processed, offline-sync archives) always stay on the local disk. + +## S3 settings + +| Variable | Meaning | +|---|---| +| `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY` | credentials | +| `AWS_DEFAULT_REGION` | region, `us-east-1` by default (any value for MinIO) | +| `AWS_BUCKET` | bucket, shared by all S3-backed disks | +| `AWS_ROOT` | optional key prefix inside the bucket | +| `AWS_ENDPOINT` | endpoint of a non-AWS service, e.g. `https://minio.example.com`; presigned links are signed for this host (see below) | +| `AWS_USE_PATH_STYLE_ENDPOINT` | `true` for MinIO and most self-hosted services | +| `AWS_TEMPORARY_URL` | optional; replaces the scheme, host and port of presigned links, only for a CDN / proxy that preserves the signed host (see below) | + +The bucket stays private: browsers get presigned links only. + +### Endpoint for presigned links + +A presigned link is signed with AWS Signature V4 for the host of `AWS_ENDPOINT`, and `Host` is part of the signature (`X-Amz-SignedHeaders=host`). The storage rejects a link that arrives with a different `Host`. + +- **Recommended:** set `AWS_ENDPOINT` to an address that both the Cattr Pods and browsers reach under the same name, for example the public URL of MinIO (`https://minio.example.com`) with `AWS_USE_PATH_STYLE_ENDPOINT=true`, and leave `AWS_TEMPORARY_URL` empty. +- `AWS_TEMPORARY_URL` does not re-sign anything: Laravel signs the link for `AWS_ENDPOINT` and then replaces only the scheme, host and port (a path in `AWS_TEMPORARY_URL` is ignored). Use it only for a CDN or reverse proxy that forwards requests to the storage with the `Host` of `AWS_ENDPOINT`. An internal endpoint such as `http://minio:9000` plus an ingress that passes the browser's public `Host` gives `SignatureDoesNotMatch`. + +## How browsers get files + +- **Screenshots.** `time-intervals/list` and `time-intervals/show` return `screenshot_url` and `thumbnail_url` that expire in **10 minutes**. On the local disk these are relative URLs of `GET /api/time-intervals/{id}/screenshot|thumb` with a temporary signature (no bearer token needed, an invalid or expired signature gives 403). These two routes are throttled per IP: `SCREENSHOTS_RATE_LIMIT` requests per minute, 600 by default, which covers a full timeline day of 1-minute screenshots. Raise it when many users share one IP (office NAT, a proxy that does not forward client addresses). On S3 they are presigned object URLs. `GET /api/time-intervals/{id}/screenshot-urls` (authenticated, same access check as viewing the interval) returns fresh URLs. The frontend calls it for report payloads that carry no URLs (dashboard, project report) and when a page stays open past the URL lifetime. +- **Attachments.** `GET /api/attachment/{id}/temporary-url?seconds=N` (1 hour by default) returns a signed app URL on the local disk and a presigned URL on S3. `N` is limited to 7 days (604800, the S3 presigning limit) on every disk; a larger value is a validation error. Links are issued only for verified (GOOD) attachments. When `cattr:attachments:verify` finds that a file no longer matches its hash, the attachment becomes BAD and its file moves to `quarantine/projects//` on the same disk. Links issued earlier stop working: the app route checks the status, and a presigned S3 URL points to a key that no longer exists. The quarantined file is kept for inspection and removed together with the attachment or its project. A BAD attachment is not verified again. +- **Reports.** The download endpoints return `/storage/reports/...` on the local disk and a presigned URL, valid for 10 minutes, on S3. The e-mail with the report attaches the file read from the `reports` disk. + +## Screenshot rotation and retention + +`cattr:screenshots:rotate` and the storage tab in the settings use the free space of the local disk. Object storage has no free space to measure, so on `FILESYSTEM_DISK=s3` rotation does nothing (`about/storage` reports `rotation_supported: false`). Set retention with bucket lifecycle rules instead, for example expire objects under `/screenshots/` (thumbnails are under `screenshots/thumbs/`) and `/reports/` after N days. Leave `attachments/` alone. Unlike local rotation, a lifecycle rule does not skip important projects, tasks or users with permanent screenshots. + +[Backend guides](README.md) · [Русский](../../ru/backend/storage.md) diff --git a/docs/developer/ru/backend/README.md b/docs/developer/ru/backend/README.md index ae43d51a8..75f7f6102 100644 --- a/docs/developer/ru/backend/README.md +++ b/docs/developer/ru/backend/README.md @@ -4,6 +4,7 @@ - [Политика доступа и роли](access-policy.md) - [События и фильтры](events-and-filters.md) +- [Хранение файлов: локальный диск или S3](storage.md) - [Создание модуля интеграции на примере Jira](jira-integration.md) - [HTTP API](https://docs.cattr.app/api/) diff --git a/docs/developer/ru/backend/storage.md b/docs/developer/ru/backend/storage.md new file mode 100644 index 000000000..a178e405c --- /dev/null +++ b/docs/developer/ru/backend/storage.md @@ -0,0 +1,44 @@ +# Хранение файлов: локальный диск или S3 + +Cattr хранит три вида файлов. Каждый из них лежит либо на локальном диске (`storage/app`, volume / PVC), либо в S3-совместимом бакете (AWS S3, MinIO и т. п.). + +| Файлы | Диск | Переменная | Локально | Префикс ключа в S3 | +|---|---|---|---|---| +| Скриншоты и миниатюры | диск по умолчанию | `FILESYSTEM_DISK=local\|s3` | `storage/app/screenshots/` | `/screenshots/` | +| Вложения задач | `attachments` | `ATTACHMENTS_DISK=local\|s3` | `storage/app/attachments/` | `/attachments/` | +| Сформированные отчёты | `reports` | `REPORTS_DISK=local\|s3` | `storage/app/public/reports/` (отдаются по `/storage`) | `/reports/` | + +Временные файлы (загружаемые скриншоты, архивы офлайн-синхронизации) всегда остаются на локальном диске. + +## Настройки S3 + +| Переменная | Назначение | +|---|---| +| `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY` | ключи доступа | +| `AWS_DEFAULT_REGION` | регион, по умолчанию `us-east-1` (для MinIO подойдёт любой) | +| `AWS_BUCKET` | бакет, общий для всех дисков на S3 | +| `AWS_ROOT` | необязательный префикс ключей внутри бакета | +| `AWS_ENDPOINT` | адрес сервиса, если это не AWS, например `https://minio.example.com`; подписанные ссылки подписываются на этот хост (см. ниже) | +| `AWS_USE_PATH_STYLE_ENDPOINT` | `true` для MinIO и большинства self-hosted сервисов | +| `AWS_TEMPORARY_URL` | необязательно; заменяет схему, хост и порт подписанных ссылок, только для CDN / прокси, который сохраняет подписанный хост (см. ниже) | + +Бакет остаётся приватным: браузер получает только подписанные ссылки. + +### Адрес для подписанных ссылок + +Подписанная ссылка подписывается AWS Signature V4 на хост из `AWS_ENDPOINT`, и `Host` входит в подпись (`X-Amz-SignedHeaders=host`). Хранилище отклоняет ссылку, пришедшую с другим `Host`. + +- **Рекомендуется:** указать в `AWS_ENDPOINT` адрес, по которому хранилище доступно под одним и тем же именем и из подов Cattr, и из браузера, например публичный адрес MinIO (`https://minio.example.com`) с `AWS_USE_PATH_STYLE_ENDPOINT=true`, а `AWS_TEMPORARY_URL` оставить пустым. +- `AWS_TEMPORARY_URL` ничего не переподписывает: Laravel подписывает ссылку на `AWS_ENDPOINT` и потом заменяет только схему, хост и порт (путь из `AWS_TEMPORARY_URL` игнорируется). Используйте его только для CDN или обратного прокси, который передаёт запросы в хранилище с `Host` из `AWS_ENDPOINT`. Внутренний адрес вроде `http://minio:9000` вместе с ingress, который передаёт публичный `Host` браузера, даёт `SignatureDoesNotMatch`. + +## Как браузер получает файлы + +- **Скриншоты.** `time-intervals/list` и `time-intervals/show` возвращают `screenshot_url` и `thumbnail_url`, которые действуют **10 минут**. На локальном диске это относительные адреса `GET /api/time-intervals/{id}/screenshot|thumb` с временной подписью (bearer-токен не нужен, неверная или просроченная подпись даёт 403). Эти два маршрута ограничены по IP: `SCREENSHOTS_RATE_LIMIT` запросов в минуту, по умолчанию 600, чего хватает на полный день таймлайна со скриншотами раз в минуту. Увеличьте значение, если много пользователей выходят с одного IP (офисный NAT, прокси без передачи адреса клиента). На S3 это подписанные ссылки на объекты. `GET /api/time-intervals/{id}/screenshot-urls` (с авторизацией, та же проверка доступа, что и для просмотра интервала) выдаёт свежие ссылки. Фронтенд вызывает его для отчётов, в данных которых ссылок нет (дашборд, отчёт по проектам), и когда страница открыта дольше срока жизни ссылки. +- **Вложения.** `GET /api/attachment/{id}/temporary-url?seconds=N` (по умолчанию час) возвращает подписанный адрес приложения на локальном диске и подписанную ссылку S3 на S3. `N` ограничен 7 днями (604800, предел подписи S3) на любом диске; большее значение даёт ошибку валидации. Ссылки выдаются только для проверенных (GOOD) вложений. Если `cattr:attachments:verify` обнаружит, что файл не совпадает со своим хешем, вложение получает статус BAD, а файл переносится в `quarantine/projects//` на том же диске. Выданные раньше ссылки перестают работать: маршрут приложения проверяет статус, а подписанная ссылка S3 указывает на ключ, которого больше нет. Файл в карантине сохраняется для разбора и удаляется вместе с вложением или его проектом. Вложение со статусом BAD повторно не проверяется. +- **Отчёты.** Эндпоинты скачивания возвращают `/storage/reports/...` на локальном диске и подписанную ссылку на 10 минут на S3. Письмо с отчётом прикладывает файл, прочитанный с диска `reports`. + +## Ротация и срок хранения скриншотов + +`cattr:screenshots:rotate` и вкладка хранилища в настройках работают от свободного места на локальном диске. У объектного хранилища свободного места нет, поэтому при `FILESYSTEM_DISK=s3` ротация ничего не делает (`about/storage` возвращает `rotation_supported: false`). Срок хранения в этом случае задают правилами жизненного цикла бакета, например удалять объекты с префиксами `/screenshots/` (миниатюры лежат в `screenshots/thumbs/`) и `/reports/` через N дней. Префикс `attachments/` не трогайте. В отличие от локальной ротации, правило бакета не делает исключений для важных проектов, задач и пользователей с постоянными скриншотами. + +[Руководства по backend](README.md) · [English](../../en/backend/storage.md) diff --git a/resources/frontend/core/components/AppImage.vue b/resources/frontend/core/components/AppImage.vue index 603239655..43f190339 100644 --- a/resources/frontend/core/components/AppImage.vue +++ b/resources/frontend/core/components/AppImage.vue @@ -35,15 +35,21 @@ diff --git a/resources/frontend/core/components/ScreenshotModal.vue b/resources/frontend/core/components/ScreenshotModal.vue index 1edf9c295..09e9258cd 100644 --- a/resources/frontend/core/components/ScreenshotModal.vue +++ b/resources/frontend/core/components/ScreenshotModal.vue @@ -7,7 +7,7 @@ @@ -101,9 +101,13 @@ import moment from 'moment-timezone'; import AppImage from './AppImage'; import { mapGetters } from 'vuex'; + import { getScreenshotUrl } from '@/services/screenshot-url.service'; + /** + * Resolves the screenshot URL of an interval (Promise); modules may override it via `config`. + */ export function screenshotPathProvider(interval) { - return `time-intervals/${interval.id}/screenshot`; + return getScreenshotUrl(interval); } export const config = { screenshotPathProvider }; @@ -142,6 +146,10 @@ computed: { ...mapGetters('user', ['companyData']), ...mapGetters('screenshots', { screenshotsEnabled: 'enabled' }), + screenshotResolver() { + const interval = this.interval; + return () => config.screenshotPathProvider(interval); + }, }, methods: { formatDate(value) { @@ -157,9 +165,6 @@ onRemove() { this.$emit('remove', this.interval.id); }, - getScreenshotPath(interval) { - return config.screenshotPathProvider(interval); - }, }, }; diff --git a/resources/frontend/core/modules/Tasks/components/Attachments.vue b/resources/frontend/core/modules/Tasks/components/Attachments.vue index b9f160a8e..822ffadb4 100644 --- a/resources/frontend/core/modules/Tasks/components/Attachments.vue +++ b/resources/frontend/core/modules/Tasks/components/Attachments.vue @@ -242,21 +242,6 @@ }, `7${this.$t('time.d')}`, ), - h( - 'AtButton', - { - props: { - type: 'primary', - size: 'smaller', - }, - on: { - click: async () => { - await this.handleTmpUrlCreation(params.item.id, 60 * 60 * 24 * 14); - }, - }, - }, - `14${this.$t('time.d')}`, - ), ]), ], ); @@ -283,7 +268,8 @@ methods: { async handleTmpUrlCreation(attachmentUUID, seconds = null) { const url = await this.taskService.generateAttachmentTmpUrl(attachmentUUID, seconds); - await this.setClipboard(window.location.origin + url); + // relative signed route on the local disk, absolute presigned URL on S3 + await this.setClipboard(new URL(url, window.location.origin).href); Vue.prototype.$Notify.success({ title: this.$i18n.t('attachments.tmp_url_created'), message: this.$i18n.t('attachments.copied_to_clipboard'), diff --git a/resources/frontend/core/services/screenshot-url.service.js b/resources/frontend/core/services/screenshot-url.service.js new file mode 100644 index 000000000..339e2877e --- /dev/null +++ b/resources/frontend/core/services/screenshot-url.service.js @@ -0,0 +1,32 @@ +import axios from 'axios'; +import { createScreenshotUrlCache, isUrlExpired } from '@/utils/screenshotUrl'; + +const cache = createScreenshotUrlCache(id => + axios.get(`time-intervals/${id}/screenshot-urls`, { muteError: true }).then(({ data }) => data.data), +); + +/** Signed routes come as "/api/...", S3 presigned URLs are absolute */ +function toAbsoluteUrl(url) { + return new URL(url, axios.defaults.baseURL || window.location.origin).href; +} + +/** + * Temporary URL of the interval screenshot (or its thumbnail) for . + * + * Uses the URL of the interval payload (time-intervals/list, time-intervals/show) while it is valid; + * otherwise (expired, or a report payload without URLs) asks the API for fresh ones. + * + * @param {Object} interval + * @param {Boolean} thumbnail + * @returns {Promise} + */ +export async function getScreenshotUrl(interval, thumbnail = false) { + const key = thumbnail ? 'thumbnail_url' : 'screenshot_url'; + + if (interval[key] && !isUrlExpired(interval[key])) { + return toAbsoluteUrl(interval[key]); + } + + const urls = await cache.get(interval.id); + return toAbsoluteUrl(urls[key]); +} diff --git a/resources/frontend/core/utils/screenshotUrl.js b/resources/frontend/core/utils/screenshotUrl.js new file mode 100644 index 000000000..5755b6f5a --- /dev/null +++ b/resources/frontend/core/utils/screenshotUrl.js @@ -0,0 +1,75 @@ +// Helpers for temporary (signed / presigned) screenshot URLs. Kept free of imports so they can be unit-tested. + +// Treat a URL as expired slightly before its real expiration (clock skew, slow image loading). +export const EXPIRY_MARGIN_MS = 60 * 1000; + +/** + * Expiration time (ms) of a temporary URL: `expires` of a Laravel signed route + * or `X-Amz-Date` + `X-Amz-Expires` of an S3 presigned URL; null when unknown. + */ +export function urlExpiresAt(url) { + let params; + try { + params = new URL(url, 'http://localhost').searchParams; + } catch (e) { + return null; + } + + if (params.has('expires')) { + const expires = Number(params.get('expires')); + return Number.isFinite(expires) ? expires * 1000 : null; + } + + const amzDate = params.get('X-Amz-Date'); + const amzExpires = Number(params.get('X-Amz-Expires')); + const match = amzDate && /^(\d{4})(\d{2})(\d{2})T(\d{2})(\d{2})(\d{2})Z$/.exec(amzDate); + if (match && Number.isFinite(amzExpires)) { + const [, y, m, d, hh, mm, ss] = match.map(Number); + return Date.UTC(y, m - 1, d, hh, mm, ss) + amzExpires * 1000; + } + + return null; +} + +/** Whether a temporary URL is expired (or its expiration is unknown) at `now` */ +export function isUrlExpired(url, now = Date.now()) { + const expiresAt = urlExpiresAt(url); + return expiresAt === null || expiresAt - EXPIRY_MARGIN_MS <= now; +} + +/** + * Cache of fresh URLs fetched per interval id, so a thumbnail and its modal share one request. + * `fetchUrls(id)` resolves to { screenshot_url, thumbnail_url, expires_in }. + */ +export function createScreenshotUrlCache(fetchUrls, now = () => Date.now()) { + const entries = new Map(); + + return { + get(id, { force = false } = {}) { + const entry = entries.get(id); + if (!force && entry && entry.validUntil > now()) { + return entry.promise; + } + + const promise = Promise.resolve(fetchUrls(id)); + const created = { promise, validUntil: Infinity }; + entries.set(id, created); + + promise.then( + urls => { + created.validUntil = now() + Number(urls.expires_in || 0) * 1000 - EXPIRY_MARGIN_MS; + }, + () => { + if (entries.get(id) === created) { + entries.delete(id); + } + }, + ); + + return promise; + }, + clear() { + entries.clear(); + }, + }; +} diff --git a/routes/api.php b/routes/api.php index 3a9deda74..2f72e474a 100644 --- a/routes/api.php +++ b/routes/api.php @@ -306,6 +306,14 @@ }); //Screenshots routes + $router->get('time-intervals/{interval}/screenshot-urls', [IntervalController::class, 'screenshotUrls']) + ->where('interval', '[0-9]+')->name('intervals.screenshot.urls'); +}); + +// Screenshot files on the local disk: no bearer token, access is granted by a temporary signed URL +// (ScreenshotService::getTemporaryUrl, relative signature checked in the controller). +// Throttled per IP by the "screenshots" limiter (AppServiceProvider), sized for a full timeline page. +Route::middleware('throttle:screenshots')->group(static function (Router $router) { $router->get('time-intervals/{interval}/screenshot', [IntervalController::class, 'showScreenshot']) ->where('interval', '[0-9]+')->name('intervals.screenshot.original'); $router->get('time-intervals/{interval}/thumb', [IntervalController::class, 'showThumbnail']) diff --git a/runtime/caddy/Caddyfile b/runtime/caddy/Caddyfile index 0506bbfeb..384963278 100644 --- a/runtime/caddy/Caddyfile +++ b/runtime/caddy/Caddyfile @@ -62,8 +62,12 @@ } route { + # Reverb does not look at the client address, so its proxy keeps Caddy's default headers. reverse_proxy @websocket 127.0.0.1:8081 + # X-Forwarded-For is overwritten with the client address resolved by trusted_proxies: behind an + # ingress Caddy would otherwise append the ingress Pod ("client, ingress") and Laravel, which + # trusts only this Caddy (127.0.0.1), would take the ingress IP for the client. reverse_proxy @php 127.0.0.1:8090 { transport http { read_timeout 300s @@ -71,12 +75,14 @@ } header_up SERVER_PORT 8080 header_up REMOTE_ADDR {client_ip} + header_up X-Forwarded-For {client_ip} } file_server @static { precompressed br gzip } + # see above: X-Forwarded-For carries only the resolved client address reverse_proxy 127.0.0.1:8090 { transport http { read_timeout 300s @@ -84,6 +90,7 @@ } header_up SERVER_PORT 8080 header_up REMOTE_ADDR {client_ip} + header_up X-Forwarded-For {client_ip} } } } diff --git a/tests/Feature/Storage/AttachmentStorageTest.php b/tests/Feature/Storage/AttachmentStorageTest.php new file mode 100644 index 000000000..92267e9db --- /dev/null +++ b/tests/Feature/Storage/AttachmentStorageTest.php @@ -0,0 +1,389 @@ +admin = UserFactory::refresh()->asAdmin()->withTokens()->create(); + /** @var Task $task */ + $task = TaskFactory::forUser($this->admin)->create(); + + $this->attachment = Attachment::create([ + 'attachmentable_id' => $task->id, + 'attachmentable_type' => Task::class, + 'original_name' => 'report final.pdf', + 'mime_type' => 'application/pdf', + 'extension' => 'pdf', + 'size' => 4, + 'hash' => '', + 'project_id' => $task->project_id, + 'user_id' => $this->admin->id, + 'status' => AttachmentStatus::GOOD, + ]); + } + + private function useS3(): void + { + config(['filesystems.disks.attachments.driver' => 's3']); + Storage::fake('attachments'); + Storage::disk('attachments')->put(app(AttachmentService::class)->getPath($this->attachment), '%PDF'); + } + + private function temporaryUrl(int $seconds): string + { + $response = $this->actingAs($this->admin) + ->get("attachment/{$this->attachment->id}/temporary-url?seconds=$seconds"); + $response->assertOk(); + + return $response->getContent(); + } + + public function test_local_disk_uses_signed_route(): void + { + Storage::fake('attachments'); + + $url = $this->temporaryUrl(30); + + $this->assertStringStartsWith("/api/tmp-attachment-link/{$this->attachment->id}?expires=", $url); + $this->assertStringContainsString('signature=', $url); + } + + public function test_s3_disk_uses_presigned_url(): void + { + $this->useS3(); + $this->freezeSecond(); + + $url = $this->temporaryUrl(3600); + + $expiration = now()->addHour()->getTimestamp(); + // Storage::fake() builds presigned-like URLs as ?expiration= + $this->assertStringEndsWith( + app(AttachmentService::class)->getPath($this->attachment) . "?expiration=$expiration", + $url, + ); + } + + public function test_s3_disk_presigns_up_to_seven_days(): void + { + $this->useS3(); + $this->freezeSecond(); + + $url = $this->temporaryUrl(AttachmentService::MAX_TEMPORARY_URL_SECONDS); + + $this->assertSame(604800, AttachmentService::MAX_TEMPORARY_URL_SECONDS); + $this->assertStringEndsWith('?expiration=' . now()->addDays(7)->getTimestamp(), $url); + } + + /** + * @return array + */ + public static function invalidLifetimes(): array + { + return [ + 'local, longer than 7 days' => ['local', AttachmentService::MAX_TEMPORARY_URL_SECONDS + 1], + 's3, longer than 7 days' => ['s3', AttachmentService::MAX_TEMPORARY_URL_SECONDS + 1], + 'local, 14 days' => ['local', 60 * 60 * 24 * 14], + 'local, zero' => ['local', 0], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('invalidLifetimes')] + public function test_lifetime_is_limited_to_seven_days_on_every_disk(string $driver, int $seconds): void + { + $driver === 's3' ? $this->useS3() : Storage::fake('attachments'); + + $this->actingAs($this->admin) + ->getJson("attachment/{$this->attachment->id}/temporary-url?seconds=$seconds") + ->assertValidationError(); + } + + /** + * @return array + */ + public static function notDownloadableStatuses(): array + { + return [ + 'local, processing' => ['local', AttachmentStatus::PROCESSING], + 'local, bad' => ['local', AttachmentStatus::BAD], + 's3, processing' => ['s3', AttachmentStatus::PROCESSING], + 's3, bad' => ['s3', AttachmentStatus::BAD], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('notDownloadableStatuses')] + public function test_temporary_url_is_issued_only_for_good_attachments( + string $driver, + AttachmentStatus $status + ): void { + $this->attachment->update(['status' => $status]); + $driver === 's3' ? $this->useS3() : Storage::fake('attachments'); + + $this->actingAs($this->admin) + ->get("attachment/{$this->attachment->id}/temporary-url?seconds=60") + ->assertNotFound('page_not_found'); + } + + public static function disks(): array + { + return ['local' => ['local'], 's3' => ['s3']]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('disks')] + public function test_temporary_url_is_issued_for_good_attachment(string $driver): void + { + $driver === 's3' ? $this->useS3() : Storage::fake('attachments'); + + $this->assertNotSame('', $this->temporaryUrl(60)); + } + + public function test_local_signed_link_does_not_serve_attachment_that_turned_bad(): void + { + Storage::fake('attachments'); + Storage::disk('attachments')->put(app(AttachmentService::class)->getPath($this->attachment), '%PDF'); + + $url = $this->temporaryUrl(60); + $this->get($url)->assertOk(); + + $this->attachment->update(['status' => AttachmentStatus::BAD]); + $this->get($url)->assertNotFound('page_not_found'); + } + + public function test_presigned_url_sets_download_headers(): void + { + $this->useS3(); + $options = null; + Storage::disk('attachments')->buildTemporaryUrlsUsing( + function ($path, $expiration, $urlOptions) use (&$options) { + $options = $urlOptions; + + return "https://bucket.example/$path"; + } + ); + + app(AttachmentService::class)->getTemporaryUrl($this->attachment, now()->addMinutes(10)); + + $this->assertSame('application/pdf', $options['ResponseContentType']); + $this->assertStringStartsWith( + 'attachment; filename="report final.pdf"', + $options['ResponseContentDisposition'] + ); + } + + /** + * Configure the attachments disk and store the file of the GOOD attachment, with a hash that no longer + * matches it, so the next verifyHash() turns the attachment BAD. + */ + private function useDiskWithTamperedFile(string $driver): void + { + if ($driver === 's3') { + $this->useS3(); + } else { + Storage::fake('attachments'); + Storage::disk('attachments')->put($this->publicKey(), '%PDF'); + } + $this->attachment->update(['hash' => 'hash of the file as uploaded']); + } + + private function publicKey(): string + { + return "projects/{$this->attachment->project_id}/{$this->attachment->id}"; + } + + private function quarantineKey(): string + { + return AttachmentService::QUARANTINE_DIR . '/' . $this->publicKey(); + } + + private function verify(): void + { + app(AttachmentService::class)->verifyHash($this->attachment); + } + + #[\PHPUnit\Framework\Attributes\DataProvider('disks')] + public function test_attachment_turning_bad_is_moved_to_quarantine(string $driver): void + { + $this->useDiskWithTamperedFile($driver); + + $this->verify(); + + $this->assertSame(AttachmentStatus::BAD, $this->attachment->fresh()->status); + Storage::disk('attachments')->assertMissing($this->publicKey()); + $this->assertSame('%PDF', Storage::disk('attachments')->get($this->quarantineKey())); + $this->assertSame($this->quarantineKey(), app(AttachmentService::class)->getPath($this->attachment->fresh())); + } + + public function test_presigned_url_issued_while_good_stops_resolving_after_turning_bad(): void + { + $this->useDiskWithTamperedFile('s3'); + $url = $this->temporaryUrl(3600); + // Storage::fake() builds presigned-like URLs as ?expiration=: the URL points to the public key + $this->assertStringContainsString($this->publicKey() . '?expiration=', $url); + Storage::disk('attachments')->assertExists($this->publicKey()); + + $this->verify(); + + // the presigned URL is signed for this key; with no object under it S3 answers 404 NoSuchKey + Storage::disk('attachments')->assertMissing($this->publicKey()); + } + + public function test_local_signed_link_issued_while_good_stops_serving_after_turning_bad(): void + { + $this->useDiskWithTamperedFile('local'); + $url = $this->temporaryUrl(3600); + $this->get($url)->assertOk(); + + $this->verify(); + + $this->get($url)->assertNotFound('page_not_found'); + } + + /** + * @return array + */ + public static function failedMoves(): array + { + return [ + 'local, move fails' => ['local', false], + 'local, copied but not deleted' => ['local', true], + 's3, move fails' => ['s3', false], + 's3, copied but not deleted' => ['s3', true], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('failedMoves')] + public function test_attachment_is_not_marked_bad_while_its_file_stays_public(string $driver, bool $copies): void + { + $this->useDiskWithTamperedFile($driver); + $fake = Storage::disk('attachments'); + // move() of S3 is copy + delete: either nothing happens, or the delete of the public key fails + Storage::set('attachments', new class ($fake->getDriver(), $fake->getAdapter(), $fake->getConfig(), $copies) + extends FilesystemAdapter { + public function __construct( + FilesystemOperator $driver, + FlysystemAdapter $adapter, + array $config, + private readonly bool $copies + ) { + parent::__construct($driver, $adapter, $config); + } + + public function move($from, $to) + { + return $this->copies && $this->copy($from, $to); + } + }); + + try { + $this->verify(); + $this->fail('The attachment has been marked BAD while its file is still public'); + } catch (RuntimeException $e) { + $this->assertStringContainsString('Unable to quarantine', $e->getMessage()); + } + + $this->assertSame(AttachmentStatus::GOOD, $this->attachment->fresh()->status); + $this->assertSame(AttachmentStatus::GOOD, $this->attachment->status); + Storage::disk('attachments')->assertExists($this->publicKey()); + } + + public function test_retry_after_the_file_was_quarantined_marks_attachment_bad(): void + { + // a previous attempt moved the file but did not save the status + $this->useDiskWithTamperedFile('s3'); + Storage::disk('attachments')->move($this->publicKey(), $this->quarantineKey()); + + $this->verify(); + + $this->assertSame(AttachmentStatus::BAD, $this->attachment->fresh()->status); + $this->assertSame('%PDF', Storage::disk('attachments')->get($this->quarantineKey())); + } + + public function test_bad_attachment_stays_bad_and_quarantined_on_reverification(): void + { + $this->useDiskWithTamperedFile('s3'); + $this->verify(); + // even a hash that matches the quarantined file again does not bring it back + $this->attachment->update(['hash' => app(AttachmentService::class)->getHashSum($this->quarantineKey())]); + + $this->verify(); + + $this->assertSame(AttachmentStatus::BAD, $this->attachment->fresh()->status); + Storage::disk('attachments')->assertMissing($this->publicKey()); + Storage::disk('attachments')->assertExists($this->quarantineKey()); + } + + #[\PHPUnit\Framework\Attributes\DataProvider('disks')] + public function test_deleting_bad_attachment_removes_quarantined_file(string $driver): void + { + $this->useDiskWithTamperedFile($driver); + $this->verify(); + Storage::disk('attachments')->assertExists($this->quarantineKey()); + + app(AttachmentService::class)->deleteAttachment($this->attachment->fresh()); + + $this->assertNull(Attachment::find($this->attachment->id)); + Storage::disk('attachments')->assertMissing($this->quarantineKey()); + } + + public function test_project_deletion_removes_quarantined_files(): void + { + $this->useDiskWithTamperedFile('s3'); + $this->verify(); + + app(AttachmentService::class)->handleProjectDeletion(Project::findOrFail($this->attachment->project_id)); + + $this->assertNull(Attachment::find($this->attachment->id)); + Storage::disk('attachments')->assertMissing($this->quarantineKey()); + } + + public function test_bad_attachment_stays_quarantined_when_its_task_changes_project(): void + { + $this->useDiskWithTamperedFile('s3'); + $this->verify(); + $oldQuarantineKey = $this->quarantineKey(); + $project = ProjectFactory::create(); + + app(AttachmentService::class)->moveAttachment($this->attachment->fresh(), $project->id); + + $this->attachment->refresh(); + $this->assertSame($project->id, $this->attachment->project_id); + Storage::disk('attachments')->assertMissing($oldQuarantineKey); + Storage::disk('attachments')->assertMissing($this->publicKey()); + $this->assertSame('%PDF', Storage::disk('attachments')->get($this->quarantineKey())); + } + + public function test_find_sus_files_looks_into_quarantine(): void + { + $this->useDiskWithTamperedFile('s3'); + $this->verify(); + $orphan = AttachmentService::QUARANTINE_DIR . "/projects/{$this->attachment->project_id}/00000000-orphan"; + Storage::disk('attachments')->put($orphan, 'orphan'); + + $this->artisan('cattr:attachments:find-sus-files')->assertSuccessful(); + + $this->assertSame([$orphan], SusFiles::pluck('path')->all()); + } +} diff --git a/tests/Feature/Storage/ReportStorageTest.php b/tests/Feature/Storage/ReportStorageTest.php new file mode 100644 index 000000000..8f67f7086 --- /dev/null +++ b/tests/Feature/Storage/ReportStorageTest.php @@ -0,0 +1,245 @@ +admin = UserFactory::refresh()->asAdmin()->create(); + } + + private function generateReport(): GenerateAndSendReport + { + $job = new GenerateAndSendReport( + DashboardExport::init([$this->admin->id], [], now()->subDay(), now(), 'UTC', 'UTC'), + $this->admin, + 'csv', + ); + $job->handle(); + + return $job; + } + + private function reportPath(): string + { + $files = Storage::disk('reports')->allFiles('reports'); + $this->assertCount(1, $files); + + return $files[0]; + } + + public function test_local_report_url_is_served_from_public_storage(): void + { + Storage::fake('reports'); + + $url = $this->generateReport()->getPublicPath(); + + // the report path starts with a slash, hence "/storage//reports/..." (unchanged behaviour) + $this->assertSame('/storage/' . $this->reportPath(), preg_replace('#/+#', '/', $url)); + } + + public function test_s3_report_url_is_presigned_for_ten_minutes(): void + { + config(['filesystems.disks.reports.driver' => 's3']); + Storage::fake('reports'); + $this->freezeSecond(); + + $url = $this->generateReport()->getPublicPath(); + + $expiration = now()->addMinutes(GenerateAndSendReport::URL_TTL_MINUTES)->getTimestamp(); + $this->assertSame(10, GenerateAndSendReport::URL_TTL_MINUTES); + // Storage::fake() builds presigned-like URLs as ?expiration= + $this->assertStringEndsWith($this->reportPath() . "?expiration=$expiration", $url); + } + + public static function reportDrivers(): array + { + return [ + 'local' => ['local'], + 's3' => ['s3'], + ]; + } + + /** + * Replace the (fake) reports disk with one whose readStream() returns $readStream($path). + */ + private function swapReportsReadStream(callable $readStream): void + { + $fake = Storage::disk('reports'); + Storage::set('reports', new class ($fake->getDriver(), $fake->getAdapter(), $fake->getConfig(), $readStream) + extends FilesystemAdapter { + /** @var callable */ + private $reader; + + public function __construct( + FilesystemOperator $driver, + FlysystemAdapter $adapter, + array $config, + callable $reader + ) { + parent::__construct($driver, $adapter, $config); + $this->reader = $reader; + } + + public function readStream($path) + { + return ($this->reader)($path); + } + }); + } + + /** + * A non-seekable read stream, like the HTTP body stream an S3 disk returns from readStream(): + * the read end of a socket pair. Remembered in $this->source to check that it gets closed. + * + * @return resource + */ + private function nonSeekableStream(string $content) + { + // the whole content has to fit into the socket buffer, the writer is closed before reading + $this->assertLessThan(64 * 1024, strlen($content)); + [$reader, $writer] = stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP); + fwrite($writer, $content); + fclose($writer); + + $this->assertFalse(stream_get_meta_data($reader)['seekable']); + $this->assertSame(-1, @fseek($reader, 0)); + + return $this->source = $reader; + } + + private function useNonSeekableReportStream(): void + { + $this->swapReportsReadStream( + fn (string $path) => $this->nonSeekableStream(Storage::disk('reports')->get($path)) + ); + } + + private function sentNotification(): ReportGenerated + { + $sent = null; + Notification::assertSentTo( + $this->admin, + ReportGenerated::class, + function (ReportGenerated $notification) use (&$sent) { + $sent = $notification; + + return true; + } + ); + + return $sent; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('reportDrivers')] + public function test_report_mail_attaches_file_from_the_reports_disk(string $driver): void + { + config(['filesystems.disks.reports.driver' => $driver]); + Storage::fake('reports'); + + $this->generateReport(); + $path = $this->reportPath(); + $stored = Storage::disk('reports')->get($path); + $this->assertNotEmpty($stored); + + if ($driver === 's3') { + // Storage::fake() is a local disk: hand bufferReport() a non-seekable stream like a real S3 disk does + $this->useNonSeekableReportStream(); + } + + $mail = $this->sentNotification()->toMail($this->admin); + $this->assertSame([], $mail->attachments); + $this->assertCount(1, $mail->rawAttachments); + $attachment = $mail->rawAttachments[0]; + $this->assertSame(basename($path), $attachment['name']); + $this->assertSame(Storage::disk('reports')->mimeType($path), $attachment['options']['mime']); + // the report is passed as a stream, not loaded into a string + $this->assertIsResource($attachment['data']); + + // render it the way MailChannel does and compare the attachment with the stored file + $email = (new Email())->from('report@example.com')->to('admin@example.com')->text('report'); + (new Message($email))->attachData($attachment['data'], $attachment['name'], $attachment['options']); + $parts = $email->getAttachments(); + $this->assertCount(1, $parts); + $this->assertSame(basename($path), $parts[0]->getFilename()); + $this->assertSame($stored, $parts[0]->getBody()); + $this->assertStringContainsString( + chunk_split(base64_encode($stored), 76, "\r\n"), + implode('', iterator_to_array($email->toIterable(), false)), + ); + + if ($driver === 's3') { + // the non-seekable source was read and closed by bufferReport() + $this->assertNotNull($this->source); + $this->assertFalse(is_resource($this->source)); + } + } + + public function test_truncated_report_stream_fails_instead_of_mailing_a_partial_report(): void + { + Storage::fake('reports'); + $this->generateReport(); + $stored = Storage::disk('reports')->get($this->reportPath()); + $this->swapReportsReadStream(fn () => $this->nonSeekableStream(substr($stored, 0, -1))); + + try { + $this->sentNotification()->toMail($this->admin); + $this->fail('A truncated report has been attached'); + } catch (RuntimeException $e) { + $this->assertStringContainsString( + sprintf('copied %d of %d bytes', strlen($stored) - 1, strlen($stored)), + $e->getMessage(), + ); + } + + $this->assertFalse(is_resource($this->source)); + } + + public function test_report_fails_when_the_buffer_cannot_be_opened(): void + { + Storage::fake('reports'); + $this->generateReport(); + $path = $this->reportPath(); + $this->useNonSeekableReportStream(); + + $notification = new class ($path, 'reports') extends ReportGenerated { + protected function openBuffer() + { + return false; + } + }; + + try { + $notification->toMail($this->admin); + $this->fail('The report has been attached without a buffer'); + } catch (RuntimeException $e) { + $this->assertStringContainsString('Unable to open a buffer', $e->getMessage()); + } + + $this->assertFalse(is_resource($this->source)); + } +} diff --git a/tests/Feature/Storage/StorageCleanerTest.php b/tests/Feature/Storage/StorageCleanerTest.php new file mode 100644 index 000000000..250c0bdce --- /dev/null +++ b/tests/Feature/Storage/StorageCleanerTest.php @@ -0,0 +1,42 @@ +assertTrue(StorageCleaner::isSupported()); + $this->assertGreaterThan(0, StorageCleaner::getFreeSpace()); + } + + public function test_rotation_is_a_noop_on_s3(): void + { + config(['filesystems.default' => 's3']); + Storage::fake('s3'); + + $this->assertFalse(StorageCleaner::isSupported()); + $this->assertEquals(0, StorageCleaner::getFreeSpace()); + $this->assertEquals(0, StorageCleaner::getUsedSpace()); + $this->assertFalse(StorageCleaner::needThinning()); + $this->assertSame(0, StorageCleaner::countAvailableScreenshots()); + StorageCleaner::thin(true); + + $this->artisan('cattr:screenshots:rotate') + ->expectsOutputToContain('bucket lifecycle rules') + ->assertSuccessful(); + + $admin = UserFactory::refresh()->asAdmin()->withTokens()->create(); + $this->actingAs($admin)->getJson('about/storage') + ->assertOk() + ->assertJsonPath('data.rotation_supported', false) + ->assertJsonPath('data.need_thinning', false); + } +} diff --git a/tests/Feature/TimeIntervals/ScreenshotTest.php b/tests/Feature/TimeIntervals/ScreenshotTest.php new file mode 100644 index 000000000..c7dcb5852 --- /dev/null +++ b/tests/Feature/TimeIntervals/ScreenshotTest.php @@ -0,0 +1,279 @@ +admin = UserFactory::refresh()->asAdmin()->withTokens()->create(); + $this->user = UserFactory::refresh()->asUser()->withTokens()->create(); + $this->interval = IntervalFactory::create(); + $this->service = app(ScreenshotService::class); + } + + private function fakeImage(): string + { + return UploadedFile::fake()->image('screenshot.jpg', 640, 360)->getContent(); + } + + private function useS3(): void + { + config(['filesystems.default' => 's3']); + Storage::fake('s3'); + } + + public function test_list_contains_signed_urls_and_no_has_screenshot(): void + { + $response = $this->actingAs($this->admin)->postJson('time-intervals/list', ['id' => $this->interval->id]); + + $response->assertOk(); + $item = $response->json('data.0') ?? $response->json('0'); + $this->assertSame($this->interval->id, $item['id']); + $this->assertArrayNotHasKey('has_screenshot', $item); + $this->assertStringStartsWith("/api/time-intervals/{$this->interval->id}/screenshot?", $item['screenshot_url']); + $this->assertStringStartsWith("/api/time-intervals/{$this->interval->id}/thumb?", $item['thumbnail_url']); + $this->assertStringContainsString('signature=', $item['thumbnail_url']); + } + + public function test_show_contains_signed_urls_and_no_has_screenshot(): void + { + $response = $this->actingAs($this->admin)->postJson('time-intervals/show', ['id' => $this->interval->id]); + + $response->assertOk(); + $response->assertJsonMissingPath('data.has_screenshot'); + $this->assertStringContainsString('signature=', $response->json('data.screenshot_url')); + $this->assertStringContainsString('signature=', $response->json('data.thumbnail_url')); + } + + public function test_model_does_not_append_urls_by_default(): void + { + $attributes = $this->interval->fresh()->toArray(); + + $this->assertArrayNotHasKey('has_screenshot', $attributes); + $this->assertArrayNotHasKey('screenshot_url', $attributes); + $this->assertArrayNotHasKey('thumbnail_url', $attributes); + } + + public function test_signed_url_ttl_is_ten_minutes(): void + { + $this->freezeSecond(); + + parse_str(parse_url($this->service->getTemporaryUrl($this->interval), PHP_URL_QUERY), $query); + + $this->assertSame(10, ScreenshotService::URL_TTL_MINUTES); + $this->assertSame(now()->addMinutes(10)->getTimestamp(), (int)$query['expires']); + } + + public function test_signed_route_serves_thumbnail_without_auth(): void + { + $image = $this->fakeImage(); + Storage::put($this->service->getThumbPath($this->interval), $image); + + $response = $this->get($this->service->getTemporaryUrl($this->interval, true)); + + $response->assertOk(); + $response->assertHeader('Content-Type', 'image/jpeg'); + $this->assertSame($image, $response->streamedContent()); + } + + public function test_signed_route_serves_screenshot_without_auth(): void + { + $image = $this->fakeImage(); + Storage::put($this->service->getScreenshotPath($this->interval), $image); + + $response = $this->get($this->service->getTemporaryUrl($this->interval)); + + $response->assertOk(); + $this->assertSame($image, $response->streamedContent()); + } + + public function test_signed_route_missing_file_is_not_found(): void + { + $this->get($this->service->getTemporaryUrl($this->interval))->assertNotFound('page_not_found'); + } + + public function test_unsigned_url_is_forbidden(): void + { + Storage::put($this->service->getThumbPath($this->interval), $this->fakeImage()); + + $this->get("time-intervals/{$this->interval->id}/thumb")->assertForbidden(); + $this->actingAs($this->admin)->get("time-intervals/{$this->interval->id}/screenshot")->assertForbidden(); + } + + public function test_signature_of_another_interval_is_forbidden(): void + { + $other = IntervalFactory::create(); + Storage::put($this->service->getThumbPath($other), $this->fakeImage()); + + $url = str_replace( + "/time-intervals/{$this->interval->id}/", + "/time-intervals/$other->id/", + $this->service->getTemporaryUrl($this->interval, true), + ); + + $this->get($url)->assertForbidden(); + } + + public function test_signed_routes_are_throttled_per_ip(): void + { + $this->assertSame(600, config('app.screenshots_rate_limit')); + config(['app.screenshots_rate_limit' => 3]); + Storage::put($this->service->getThumbPath($this->interval), $this->fakeImage()); + Storage::put($this->service->getScreenshotPath($this->interval), $this->fakeImage()); + + // thumbnails and screenshots share one budget + $this->get($this->service->getTemporaryUrl($this->interval, true))->assertOk() + ->assertHeader('X-RateLimit-Limit', 3); + $this->get($this->service->getTemporaryUrl($this->interval))->assertOk(); + $this->get($this->service->getTemporaryUrl($this->interval, true))->assertOk() + ->assertHeader('X-RateLimit-Remaining', 0); + + // Asserted as the exception: App\Exceptions\Handler currently renders HTTP exceptions that are not + // responder ones (ThrottleRequestsException, InvalidSignatureException) as 500, fixed separately. + $this->withoutExceptionHandling(); + try { + $this->get($this->service->getTemporaryUrl($this->interval)); + $this->fail('The 4th request within a minute must be throttled'); + } catch (ThrottleRequestsException $e) { + $this->assertSame(429, $e->getStatusCode()); + } + + // another client is not affected + $this->withServerVariables(['REMOTE_ADDR' => '10.0.0.2']) + ->get($this->service->getTemporaryUrl($this->interval, true))->assertOk(); + + $this->travel(61)->seconds(); + $this->withServerVariables(['REMOTE_ADDR' => '127.0.0.1']) + ->get($this->service->getTemporaryUrl($this->interval, true))->assertOk(); + } + + public function test_expired_url_is_forbidden(): void + { + Storage::put($this->service->getThumbPath($this->interval), $this->fakeImage()); + $url = $this->service->getTemporaryUrl($this->interval, true); + + $this->travel(9)->minutes(); + $this->get($url)->assertOk(); + + $this->travel(2)->minutes(); + $this->get($url)->assertForbidden(); + } + + public function test_screenshot_urls_endpoint(): void + { + $response = $this->actingAs($this->admin)->getJson("time-intervals/{$this->interval->id}/screenshot-urls"); + + $response->assertOk(); + $response->assertJsonPath('data.expires_in', 600); + $this->assertStringContainsString('signature=', $response->json('data.screenshot_url')); + $this->assertStringContainsString('signature=', $response->json('data.thumbnail_url')); + } + + public function test_screenshot_urls_endpoint_requires_access(): void + { + $this->getJson("time-intervals/{$this->interval->id}/screenshot-urls")->assertUnauthorized(); + // intervals outside the user's access scope are not even found, as with time-intervals/show + $this->actingAs($this->user)->getJson("time-intervals/{$this->interval->id}/screenshot-urls") + ->assertNotFound('page_not_found'); + } + + private function createIntervalWithScreenshot(): TimeInterval + { + /** @var Task $task */ + $task = TaskFactory::forUser($this->admin)->create(); + $task->project->screenshots_state = ScreenshotsState::REQUIRED; + $task->project->save(); + + $data = IntervalFactory::createRandomModelData(); + $data['task_id'] = $task->id; + $data['user_id'] = $this->admin->id; + $data['screenshot'] = UploadedFile::fake()->image('screenshot.jpg', 1280, 720); + + $response = $this->actingAs($this->admin) + ->post('time-intervals/create', $data, ['Accept' => 'application/json']); + $response->assertOk(); + + return TimeInterval::findOrFail($response->json('data.id')); + } + + public function test_upload_stores_screenshot_and_thumbnail_on_local_disk(): void + { + $interval = $this->createIntervalWithScreenshot(); + + Storage::disk('local')->assertExists($this->service->getScreenshotPath($interval)); + Storage::disk('local')->assertExists($this->service->getThumbPath($interval)); + $this->assertSame([], Storage::disk('local')->files('tmp')); + } + + public function test_upload_stores_screenshot_and_thumbnail_on_s3(): void + { + $this->useS3(); + + $interval = $this->createIntervalWithScreenshot(); + + Storage::disk('s3')->assertExists($this->service->getScreenshotPath($interval)); + Storage::disk('s3')->assertExists($this->service->getThumbPath($interval)); + // the upload itself is only a temporary local copy + Storage::disk('s3')->assertMissing('tmp'); + Storage::disk('local')->assertMissing($this->service->getScreenshotPath($interval)); + + [$width] = getimagesizefromstring(Storage::disk('s3')->get($this->service->getThumbPath($interval))); + $this->assertSame(280, $width); + } + + public function test_thumbnail_is_generated_from_s3(): void + { + $this->useS3(); + Storage::disk('s3')->put($this->service->getScreenshotPath($this->interval), $this->fakeImage()); + + $this->service->createThumbnail($this->interval); + + Storage::disk('s3')->assertExists($this->service->getThumbPath($this->interval)); + } + + public function test_s3_payload_contains_presigned_urls(): void + { + $this->useS3(); + $this->freezeSecond(); + + $response = $this->actingAs($this->admin)->postJson('time-intervals/show', ['id' => $this->interval->id]); + + $response->assertOk(); + $expiration = now()->addMinutes(10)->getTimestamp(); + // Storage::fake() builds presigned-like URLs as ?expiration= + $this->assertStringEndsWith( + $this->service->getScreenshotPath($this->interval) . "?expiration=$expiration", + $response->json('data.screenshot_url'), + ); + $this->assertStringEndsWith( + $this->service->getThumbPath($this->interval) . "?expiration=$expiration", + $response->json('data.thumbnail_url'), + ); + } +} diff --git a/tests/Feature/TrustedProxiesTest.php b/tests/Feature/TrustedProxiesTest.php new file mode 100644 index 000000000..760c1f93f --- /dev/null +++ b/tests/Feature/TrustedProxiesTest.php @@ -0,0 +1,83 @@ + $request->ip()); + } + + private function clientIp(string $remoteAddr, ?string $forwardedFor = null): string + { + $request = $this->withServerVariables(['REMOTE_ADDR' => $remoteAddr]); + if ($forwardedFor !== null) { + $request = $request->withHeader('X-Forwarded-For', $forwardedFor); + } + + return $request->get('_test/client-ip')->assertOk()->getContent(); + } + + public function test_forwarded_client_is_used_behind_caddy(): void + { + $this->assertSame('203.0.113.5', $this->clientIp(self::CADDY, '203.0.113.5')); + } + + public function test_without_forwarded_header_the_peer_is_the_client(): void + { + $this->assertSame(self::CADDY, $this->clientIp(self::CADDY)); + } + + public function test_forwarded_header_from_untrusted_peer_is_ignored(): void + { + $this->assertSame('198.51.100.7', $this->clientIp('198.51.100.7', '203.0.113.5')); + $this->assertSame('10.0.0.2', $this->clientIp('10.0.0.2', '203.0.113.5')); + } + + public function test_screenshot_limiter_is_keyed_by_forwarded_client(): void + { + Storage::fake('local'); + config(['app.screenshots_rate_limit' => 2]); + /** @var TimeInterval $interval */ + $interval = IntervalFactory::create(); + $service = app(ScreenshotService::class); + Storage::put($service->getThumbPath($interval), UploadedFile::fake()->image('t.jpg', 32, 18)->getContent()); + $url = $service->getTemporaryUrl($interval, true); + + $viaCaddy = fn (string $client) => $this->withServerVariables(['REMOTE_ADDR' => self::CADDY]) + ->withHeader('X-Forwarded-For', $client) + ->get($url); + + $viaCaddy('203.0.113.5')->assertOk(); + $viaCaddy('203.0.113.5')->assertOk()->assertHeader('X-RateLimit-Remaining', 0); + + // a different client behind the same proxy has its own budget + $viaCaddy('203.0.113.6')->assertOk()->assertHeader('X-RateLimit-Remaining', 1); + + $this->withoutExceptionHandling(); + try { + $viaCaddy('203.0.113.5'); + $this->fail('The exhausted client must be throttled'); + } catch (ThrottleRequestsException $e) { + $this->assertSame(429, $e->getStatusCode()); + } + } +} diff --git a/tests/js/screenshotUrl.test.cjs b/tests/js/screenshotUrl.test.cjs new file mode 100644 index 000000000..67d890cee --- /dev/null +++ b/tests/js/screenshotUrl.test.cjs @@ -0,0 +1,82 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const test = require('node:test'); +const vm = require('node:vm'); + +// The util is an ES module for the webpack build; strip the export keywords and +// evaluate it in an isolated context so the test exercises the production source. +function loadUtil() { + const source = fs.readFileSync( + path.join(__dirname, '../../resources/frontend/core/utils/screenshotUrl.js'), + 'utf8', + ); + const script = source.replace(/^export /gm, ''); + const sandbox = { URL, Date, Number, Promise, Map, Infinity }; + vm.runInNewContext( + `${script}\nthis.util = { EXPIRY_MARGIN_MS, urlExpiresAt, isUrlExpired, createScreenshotUrlCache };`, + sandbox, + ); + return sandbox.util; +} + +const { EXPIRY_MARGIN_MS, urlExpiresAt, isUrlExpired, createScreenshotUrlCache } = loadUtil(); + +test('expiration of a Laravel signed route', () => { + assert.equal(urlExpiresAt('/api/time-intervals/1/thumb?expires=1700000000&signature=abc'), 1700000000000); +}); + +test('expiration of an S3 presigned URL', () => { + const url = + 'https://s3.example/bucket/screenshots/a.jpg?X-Amz-Algorithm=AWS4-HMAC-SHA256' + + '&X-Amz-Date=20261025T120000Z&X-Amz-Expires=600&X-Amz-Signature=abc'; + assert.equal(urlExpiresAt(url), Date.UTC(2026, 9, 25, 12, 10, 0)); +}); + +test('unknown expiration', () => { + assert.equal(urlExpiresAt('/api/time-intervals/1/thumb'), null); + assert.equal(isUrlExpired('/api/time-intervals/1/thumb'), true); + assert.equal(isUrlExpired(null), true); +}); + +test('expired with a safety margin', () => { + const url = '/x?expires=1000'; + assert.equal(isUrlExpired(url, 1000 * 1000 - EXPIRY_MARGIN_MS - 1), false); + assert.equal(isUrlExpired(url, 1000 * 1000 - EXPIRY_MARGIN_MS), true); +}); + +test('cache shares one request per interval until the URLs expire', async () => { + let now = 0; + const calls = []; + const cache = createScreenshotUrlCache( + id => { + calls.push(id); + return Promise.resolve({ thumbnail_url: `t${calls.length}`, expires_in: 600 }); + }, + () => now, + ); + + const [a, b] = await Promise.all([cache.get(1), cache.get(1)]); + assert.equal(a, b); + assert.deepEqual(calls, [1]); + + now = 600 * 1000 - EXPIRY_MARGIN_MS - 1; + await cache.get(1); + assert.deepEqual(calls, [1]); + + now += 1; + assert.equal((await cache.get(1)).thumbnail_url, 't2'); + assert.equal((await cache.get(1, { force: true })).thumbnail_url, 't3'); + assert.deepEqual(calls, [1, 1, 1]); +}); + +test('cache forgets failed requests', async () => { + let fail = true; + const cache = createScreenshotUrlCache(() => + fail ? Promise.reject(new Error('nope')) : Promise.resolve({ expires_in: 600, screenshot_url: 's' }), + ); + + await assert.rejects(cache.get(7)); + fail = false; + assert.equal((await cache.get(7)).screenshot_url, 's'); +});