mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
denkfabrik-li
1ed29ec072
Both preference writers validated their array as ['required', 'array']
and looped updateOrCreate over it:
'widgets' => ['required', 'array'],
'widgets.*.widget_key' => ['required', 'string', Rule::in(WIDGET_KEYS)],
Rule::in answers "is this a key I know", once per element. It says
nothing about how many elements there are, and nothing about whether they
repeat -- so a request could name the same valid key any number of times
and buy a SELECT and an UPDATE for each one.
Measured on this base, sent as JSON (a form-encoded array that size is
truncated by max_input_vars long before it reaches the controller):
widgets 10 entries 37 queries 1 row
500 entries 1044 queries 1 row
3000 entries 7051 queries 1 row
notifications 10 entries 23 queries 1 row
2000 entries 2025 queries 1 row
One row, every time. The work is not even data growth -- 3000 entries
write the same single row 3000 times, because updateOrCreate matches on
(user_id, widget_key) and every element after the first is an update of
what the one before it just wrote.
Neither route is behind a throttle: bootstrap/app.php applies
throttleApi() to the API group only, /dashboard/widgets is behind `auth`
alone and /settings/notifications is deliberately outside the `staff`
group, since every account manages its own. So the weakest account on the
installation -- a client with no permission at all -- can reach both, and
the only ceiling is post_max_size.
Both are bounded by the list they already validate against, not by a
number:
- widgets by count(self::WIDGET_KEYS), the same constant Rule::in reads.
- preferences by count($this->emailableKeys()), because
NotificationTypeRegistry is deliberately open -- "never a closed enum,
since core must not need to know a package's notification type keys at
compile time" -- so a literal would be wrong the day a module
registers one.
`distinct` on the key does the other half: a layout has at most one entry
per widget, which is what the screen sends and what the loop assumes.
After: 3000 entries cost 30 queries and write nothing, refused with a 422
instead of half-applied.
Two findings, one cause, one change -- they are the same three words in
two modules, and splitting them would leave the rule stated once and
broken once. Tests live with each controller: two refusals each, both
failing against the unfixed controllers, plus one for the largest
legitimate submission -- a full nine-widget layout, and every emailable
type at once -- so the bound can never be tighter than the screen.
77 lines
2.8 KiB
PHP
77 lines
2.8 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Audit\Http\Controllers;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Modules\Audit\Models\DashboardWidgetPreference;
|
|
use Illuminate\Http\RedirectResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Validation\Rule;
|
|
|
|
/**
|
|
* Saves a staff member's dashboard layout (per-widget enabled/column/
|
|
* position, plus column count) — every account manages their own, same
|
|
* "self-scoped, no extra permission needed" shape as
|
|
* NotificationPreferencesController. Which widgets a viewer can even
|
|
* toggle is enforced entirely by DashboardController's own permission
|
|
* checks on read — this endpoint only ever stores a preference, it never
|
|
* grants visibility a viewer's Gates don't already allow.
|
|
*/
|
|
class DashboardWidgetPreferencesController extends Controller
|
|
{
|
|
/**
|
|
* Every key DashboardController::__invoke() knows how to render —
|
|
* kept here as the single validation allowlist so a stray/typo'd key
|
|
* can't accumulate a dead row.
|
|
*/
|
|
private const WIDGET_KEYS = [
|
|
'counters',
|
|
'transfers',
|
|
'top_clients_by_storage',
|
|
'largest_files',
|
|
'recent',
|
|
'system',
|
|
'news',
|
|
'expired_files',
|
|
'api',
|
|
];
|
|
|
|
public function update(Request $request): RedirectResponse
|
|
{
|
|
$user = $request->user();
|
|
assert($user !== null);
|
|
|
|
$validated = $request->validate([
|
|
'columns' => ['required', 'integer', 'between:1,4'],
|
|
// Bounded by the allowlist itself, and unique on the key. The
|
|
// Rule::in below checks each value; it says nothing about how
|
|
// many there are or whether they repeat, and the loop writes
|
|
// one row per element. A layout has at most one entry per
|
|
// widget, so anything longer than the registry is not a layout
|
|
// this screen could have produced.
|
|
'widgets' => ['required', 'array', 'max:'.count(self::WIDGET_KEYS)],
|
|
'widgets.*.widget_key' => ['required', 'string', 'distinct', Rule::in(self::WIDGET_KEYS)],
|
|
'widgets.*.enabled' => ['required', 'boolean'],
|
|
'widgets.*.column_index' => ['required', 'integer', 'between:0,3'],
|
|
'widgets.*.position' => ['required', 'integer', 'min:0'],
|
|
]);
|
|
|
|
foreach ($validated['widgets'] as $widget) {
|
|
DashboardWidgetPreference::query()->updateOrCreate(
|
|
['user_id' => $user->id, 'widget_key' => $widget['widget_key']],
|
|
[
|
|
'enabled' => $widget['enabled'],
|
|
'column_index' => $widget['column_index'],
|
|
'position' => $widget['position'],
|
|
],
|
|
);
|
|
}
|
|
|
|
$user->update(['dashboard_columns' => $validated['columns']]);
|
|
|
|
return back();
|
|
}
|
|
}
|