Files
projectsend/app/Modules/Notifications/Http/Controllers/NotificationPreferencesController.php
denkfabrik-li 1ed29ec072 Bound the two preference endpoints by their own registries
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.
2026-08-28 23:53:12 +02:00

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());
}
}