mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-16 16:45:07 +00:00
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.
105 lines
3.7 KiB
PHP
105 lines
3.7 KiB
PHP
<?php
|
|
|
|
declare(strict_types=1);
|
|
|
|
namespace App\Modules\Notifications\Http\Controllers;
|
|
|
|
use App\Http\Controllers\Controller;
|
|
use App\Modules\Notifications\NotificationPreference;
|
|
use App\Modules\Notifications\NotificationPreferences;
|
|
use App\Modules\Notifications\NotificationTypeDefinition;
|
|
use App\Modules\Notifications\NotificationTypeRegistry;
|
|
use Illuminate\Http\RedirectResponse;
|
|
use Illuminate\Http\Request;
|
|
use Illuminate\Validation\Rule;
|
|
use Inertia\Inertia;
|
|
use Inertia\Response;
|
|
|
|
/**
|
|
* Per-user "also email me for this" toggles — every account (staff or
|
|
* client) manages their own, hence living under settings/* alongside
|
|
* profile/password/two-factor rather than system/settings/*.
|
|
*/
|
|
class NotificationPreferencesController extends Controller
|
|
{
|
|
public function __construct(
|
|
private readonly NotificationTypeRegistry $types,
|
|
private readonly NotificationPreferences $preferences,
|
|
) {}
|
|
|
|
public function edit(Request $request): Response
|
|
{
|
|
$user = $request->user();
|
|
assert($user !== null);
|
|
|
|
return Inertia::render('settings/notifications', [
|
|
'types' => array_map(fn (NotificationTypeDefinition $type): array => [
|
|
'key' => $type->key,
|
|
'label' => $type->label,
|
|
'email_enabled' => $this->preferences->emailEnabledFor($user, $type),
|
|
], $this->emailable()),
|
|
]);
|
|
}
|
|
|
|
public function update(Request $request): RedirectResponse
|
|
{
|
|
$user = $request->user();
|
|
assert($user !== null);
|
|
|
|
$keys = $this->emailableKeys();
|
|
|
|
$validated = $request->validate([
|
|
// Bounded by the registry, and unique on the type. 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. The count comes from the registry
|
|
// rather than a literal because the registry is open --
|
|
// modules register their own types into it, so a number here
|
|
// would be wrong the moment one does.
|
|
'preferences' => ['required', 'array', 'max:'.count($keys)],
|
|
// Against the registry, not merely "a string": a preference row
|
|
// for a type nothing can send is a row that will never be read
|
|
// again, and the screen only ever offers back what edit() gave
|
|
// it.
|
|
'preferences.*.type' => ['required', 'string', 'distinct', Rule::in($keys)],
|
|
'preferences.*.email_enabled' => ['required', 'boolean'],
|
|
]);
|
|
|
|
foreach ($validated['preferences'] as $preference) {
|
|
NotificationPreference::query()->updateOrCreate(
|
|
['user_id' => $user->id, 'type' => $preference['type']],
|
|
['email_enabled' => $preference['email_enabled']],
|
|
);
|
|
}
|
|
|
|
return back();
|
|
}
|
|
|
|
/**
|
|
* Only types that can email at all have anything to opt in or out of —
|
|
* a pure in-app type has no toggle to show. Either route counts:
|
|
* Notifier sending a mail class directly, or the digest buffering and
|
|
* sending one.
|
|
*
|
|
* Shared by both halves on purpose, so what the screen offers and what
|
|
* it accepts back cannot drift apart.
|
|
*
|
|
* @return list<NotificationTypeDefinition>
|
|
*/
|
|
private function emailable(): array
|
|
{
|
|
return array_values(array_filter(
|
|
$this->types->all(),
|
|
fn (NotificationTypeDefinition $type): bool => $type->mailNotification !== null || $type->digestMail !== null,
|
|
));
|
|
}
|
|
|
|
/**
|
|
* @return list<string>
|
|
*/
|
|
private function emailableKeys(): array
|
|
{
|
|
return array_map(fn (NotificationTypeDefinition $type): string => $type->key, $this->emailable());
|
|
}
|
|
}
|