mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 09:05:08 +00:00
ignacionelson
57540164fa
Two routes still assumed every file sits on local disk, which stopped
being true the moment external storage was switched on. A share link
answered with X-Accel-Redirect whatever the file's disk said, pointing
nginx at a path it has nothing behind; a public listing built a
thumbnail from Storage::disk('files')->path(), which for an externally
stored file is a path nobody ever wrote. Both fail only for installs
using S3, and only on those two routes, so the same file downloading
correctly from the file manager made the share link look like the
broken thing rather than where the file lives.
Neither is a new rule. FileDownloadController and
FileThumbnailController already did it right, which is the actual
finding: the knowledge was sitting in a private method on one class and
inline in another, so the next caller could not inherit it and did not.
Both are now objects with one job.
StoredFileResponse replaces InlineFileResponse and grows an
attachment() alongside inline(), since the two differ only by
disposition. LocalSourceFile takes a closure rather than returning a
path: the version that returned one also left the caller to unlink it,
and both of those are exactly the mistakes made here.
The regression tests fail against the previous controllers — checked in
both directions rather than assumed.
48 lines
1.6 KiB
PHP
48 lines
1.6 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Files\Http\Controllers;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Modules\Audit\Action;
|
|
use App\Modules\Audit\ActivityLogger;
|
|
use App\Modules\Files\Access\DownloadAllowance;
|
|
use App\Modules\Files\Delivery\StoredFileResponse;
|
|
use App\Modules\Files\Models\File;
|
|
use Illuminate\Http\RedirectResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Http\Response;
|
|
use Illuminate\Support\Facades\Gate;
|
|
|
|
/**
|
|
* Authorized downloads without the bytes ever traversing PHP: the app
|
|
* checks the policy, and StoredFileResponse answers with either an
|
|
* X-Accel-Redirect for nginx to stream from the protected location
|
|
* (brief §3) or a presigned URL when the file lives on external storage,
|
|
* since nginx has no way to serve bytes it doesn't have on disk.
|
|
*/
|
|
class FileDownloadController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly ActivityLogger $activity,
|
|
private readonly DownloadAllowance $allowance,
|
|
private readonly StoredFileResponse $bytes,
|
|
) {}
|
|
|
|
public function __invoke(Request $request, File $file): Response|RedirectResponse
|
|
{
|
|
Gate::authorize('view', $file);
|
|
|
|
// Separate from the policy on purpose: a spent download limit is
|
|
// not "you may not see this file" — the file stays listed, and
|
|
// the same person may still open its details. It is only the
|
|
// taking of a copy that stops. See DownloadAllowance.
|
|
abort_unless($this->allowance->allows($file, $request->user()), 403);
|
|
|
|
$this->activity->log(Action::FileDownloaded, subject: $file);
|
|
|
|
return $this->bytes->attachment($file);
|
|
}
|
|
}
|