mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 09:05:08 +00:00
3917cb2af3
Laravel's `date` rule accepts a JSON number when it reads as a real day
(20301231 passes) and hands it on unconverted. Every file expiry field
then passes it to a method that only takes a string, so the request
failed with a 500 instead of a validation error.
Affected: PATCH /api/v1/files/{file}, the staff file editor, the bulk
editor, new share links and the client file editor. Each now also
requires `string`, so a number is a 422 on `expires_at`. Dates sent as
text behave exactly as before. A form never sent a number, so this was
only reachable with a hand-written JSON body.
124 lines
5.3 KiB
PHP
124 lines
5.3 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\Models\File;
|
|
use App\Modules\Files\Models\ShareLink;
|
|
use App\Modules\Files\Sharing\CreateShareLink;
|
|
use App\Modules\Platform\Localization\LocalDay;
|
|
use App\Modules\Platform\Localization\TimezoneRegistry;
|
|
use Illuminate\Http\RedirectResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Support\Facades\Gate;
|
|
use Illuminate\Support\Str;
|
|
use Illuminate\Validation\Rule;
|
|
use Illuminate\Validation\ValidationException;
|
|
|
|
/**
|
|
* Public share links for a file — same "can share this" gate as
|
|
* assigning to a client or group (Gate::update), not a dedicated
|
|
* permission. The expiry/download-limit fields are separately gated by
|
|
* the set_file_expiration_date/limit_downloads permissions: without
|
|
* them the field is simply absent, not a 403.
|
|
*/
|
|
class ShareLinksController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly ActivityLogger $activity,
|
|
private readonly TimezoneRegistry $timezones,
|
|
private readonly CreateShareLink $links,
|
|
) {}
|
|
|
|
public function store(Request $request, File $file): RedirectResponse
|
|
{
|
|
Gate::authorize('update', $file);
|
|
|
|
$validated = $request->validate([
|
|
// Deliberately not `after:now`: that rule reads the bare
|
|
// YYYY-MM-DD the picker posts as midnight UTC, so a creator
|
|
// far enough east would be told today's date is in the past
|
|
// while it is plainly still today where they are. The check
|
|
// moves below, onto the instant the date actually resolves to.
|
|
'expires_at' => ['nullable', 'string', 'date'],
|
|
'max_downloads' => ['nullable', 'integer', 'min:1'],
|
|
// A custom token is optional — leave blank for a random one,
|
|
// same as before. Must not collide with the file's own
|
|
// public slug: the two live in different URL namespaces
|
|
// (/s/{token} vs the public group listing), but sharing the
|
|
// same string between them is confusing enough to reject.
|
|
// The token IS the authorization for /s/{token} — there is no
|
|
// second factor behind it — so it has to be long enough not to
|
|
// be guessable. 6 chars of [A-Za-z0-9_-] is ~36 bits, within
|
|
// reach of sustained guessing (the route's 30/min IP throttle
|
|
// is not a bound when the attacker has many IPs). Random tokens
|
|
// are Str::random(32); 12 is the floor for a chosen one.
|
|
'token' => ['nullable', 'string', 'min:12', 'max:64', 'regex:/^[A-Za-z0-9_-]+$/', Rule::unique('share_links', 'token')],
|
|
]);
|
|
|
|
if (($validated['token'] ?? null) !== null && $validated['token'] === $file->slug) {
|
|
throw ValidationException::withMessages([
|
|
'token' => __('This link cannot match the file\'s own public URL slug.'),
|
|
]);
|
|
}
|
|
|
|
$user = $request->user();
|
|
assert($user !== null);
|
|
|
|
// End of that day in the creator's own zone — a link "expiring on
|
|
// the 12th" stays usable through the 12th, which is what they
|
|
// will have told the recipient.
|
|
$expiresAt = ($validated['expires_at'] ?? null) === null
|
|
? null
|
|
: LocalDay::end($validated['expires_at'], $this->timezones->resolve($user));
|
|
|
|
if ($expiresAt !== null && $expiresAt->isPast()) {
|
|
throw ValidationException::withMessages([
|
|
'expires_at' => __('The expiry date must be in the future.'),
|
|
]);
|
|
}
|
|
|
|
// The permission gates stay here, where the request is: whether
|
|
// this person may set an expiry or a cap is a fact about them,
|
|
// not about link creation, and the action has no viewer to ask.
|
|
$this->links->for(
|
|
file: $file,
|
|
creator: $user,
|
|
expiresAt: $user->can('set_file_expiration_date') ? $expiresAt : null,
|
|
// Cast, and null kept as null rather than falling through a
|
|
// bare (int) that would turn "no cap" into a cap of zero. The
|
|
// `integer` rule validates a numeric string without converting
|
|
// it, and this file is strict_types, so an uncast "5" is a
|
|
// TypeError against `?int $maxDownloads`. Nothing sends one
|
|
// today only because files/edit.tsx calls Number() first --
|
|
// which is a fact about a frontend file, not a guarantee this
|
|
// signature has. It cost a 500 on the client form, where the
|
|
// same field was typed as a string.
|
|
maxDownloads: $user->can('limit_downloads') && ($validated['max_downloads'] ?? null) !== null
|
|
? (int) $validated['max_downloads']
|
|
: null,
|
|
token: $validated['token'] ?? null,
|
|
);
|
|
|
|
return back()->with('success', __('Public link created.'));
|
|
}
|
|
|
|
public function destroy(ShareLink $shareLink): RedirectResponse
|
|
{
|
|
$file = $shareLink->shareable;
|
|
abort_unless($file instanceof File, 404);
|
|
|
|
Gate::authorize('update', $file);
|
|
|
|
$shareLink->delete();
|
|
|
|
$this->activity->log(Action::ShareLinkRevoked, subject: $file);
|
|
|
|
return back()->with('success', __('Public link revoked.'));
|
|
}
|
|
}
|