fix: address code review findings on PWA/push feature

- serwist route: force useNativeEsbuild to avoid depending on the
  esbuild-wasm auto-detection fallback
- InstallPrompt/NotificationPrompt: switch to useSyncExternalStore so
  browser-dependent eligibility no longer causes a hydration mismatch
  and no longer sets state synchronously inside an effect body
- NotificationPrompt: a failed subscribe/server-registration no longer
  permanently dismisses the prompt (localStorage), keeping an in-app
  retry path; a granted-but-unregistered subscription is detected via
  pushManager.getSubscription() and re-offers the prompt
- send-push: bound each webpush.sendNotification call with a timeout
  so an unresponsive endpoint can't leave the settle indefinitely
- push-subscription-actions: validate endpoint against an allowlist of
  known push service origins before persisting (SSRF hardening), and
  scope unsubscribeFromPush to the authenticated recipient
- create-notification: wrap the push side-effect in next/server's
  after() so a serverless function isn't frozen mid-delivery
- DiscountCodeList: disable close/cancel controls while a save is
  in flight

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Rajpreet0 2026-09-14 16:49:07 +02:00
parent c7da35649c
commit 711b02dffb
7 changed files with 180 additions and 44 deletions

View file

@ -9,5 +9,10 @@ export const { dynamic, dynamicParams, revalidate, generateStaticParams, GET } =
{
swSrc: "src/app/sw.ts",
additionalPrecacheEntries: [{ url: "/manifest.json", revision }],
// Default ist nur auf win32 true (siehe @serwist/turbopack), auf
// Linux/macOS sonst esbuild-wasm - explizit erzwungen, damit das
// Verhalten plattformunabhängig gleich ist (esbuild bleibt außerdem als
// direkte Dependency installiert, esbuild-wasm nur als Fallback-Netz).
useNativeEsbuild: true,
}
);

View file

@ -1,6 +1,6 @@
"use client";
import { useEffect, useState } from "react";
import { useEffect, useState, useSyncExternalStore } from "react";
import { X, Share, Plus, Download } from "lucide-react";
import { Button } from "@/src/components/ui/button";
import {
@ -18,18 +18,33 @@ function isIos(): boolean {
return /iphone|ipad|ipod/i.test(window.navigator.userAgent);
}
// Lazy-Initializer statt Effect: eligible/showIosHint hängen nur von
// Browser-APIs ab, nicht von React-Zustand, der synchronisiert werden müsste.
// Serverseitig (kein window) liefert das konsistent false, identisch zum
// allerersten Client-Render - kein Hydration-Mismatch.
function computeEligible(): boolean {
if (typeof window === "undefined") return false;
return isInstallPromptPending();
// Kein echtes externes Event zum Abonnieren - subscribe registriert daher
// nie ein Update. Der Witz ist nur getServerSnapshot: React rendert serverseitig
// (und im allerersten Client-Pass vor der Hydration) "false" und ersetzt es
// direkt danach durch den echten Browser-Wert, OHNE die
// react-hooks/set-state-in-effect-Regel zu verletzen und OHNE eine
// Hydration-Mismatch-Warnung auszulösen (dafür ist der Hook gebaut).
function subscribeNever() {
return () => {};
}
export function InstallPrompt() {
const [eligible, setEligible] = useState(computeEligible);
const [showIosHint] = useState(() => eligible && isIos());
// useSyncExternalStore statt Lazy-useState/Effect: getServerSnapshot liefert
// serverseitig UND im allerersten Client-Pass vor der Hydration "false",
// React ersetzt es danach durch den echten Browser-Wert - ohne
// Hydration-Mismatch-Warnung und ohne die react-hooks/set-state-in-effect-
// Regel zu verletzen (kein setState im Effect-Body nötig).
const pending = useSyncExternalStore(subscribeNever, isInstallPromptPending, () => false);
const showIosHint = useSyncExternalStore(
subscribeNever,
() => pending && isIos(),
() => false
);
// dismiss() muss den Prompt sofort ausblenden können, auch bevor sich der
// zugrunde liegende localStorage-Wert das nächste Mal ändert - pending
// allein kann das nicht (useSyncExternalStore hat kein echtes Update-Event).
const [dismissed, setDismissed] = useState(false);
const eligible = pending && !dismissed;
const [deferredPrompt, setDeferredPrompt] = useState<BeforeInstallPromptEvent | null>(null);
useEffect(() => {
@ -45,7 +60,7 @@ export function InstallPrompt() {
const dismiss = () => {
localStorage.setItem(INSTALL_DISMISSED_KEY, "1");
setEligible(false);
setDismissed(true);
window.dispatchEvent(new Event(INSTALL_PROMPT_SETTLED_EVENT));
};

View file

@ -1,6 +1,6 @@
"use client";
import { useEffect, useState } from "react";
import { useEffect, useState, useSyncExternalStore } from "react";
import { X, BellRing } from "lucide-react";
import { Button } from "@/src/components/ui/button";
import { INSTALL_PROMPT_SETTLED_EVENT, isInstallPromptPending } from "@/src/lib/pwa";
@ -29,14 +29,39 @@ function isPushSupported(): boolean {
);
}
// Lazy-Initializer statt Effect (siehe InstallPrompt.tsx): Browser-API-Werte
// werden serverseitig identisch mit dem allerersten Client-Render (false)
// beantwortet, kein Hydration-Mismatch möglich.
function computeEligible(): boolean {
// Basis-Voraussetzungen, die sich zur Laufzeit nicht ändern (API-Support,
// VAPID-Konfiguration) - anders als Notification.permission/localStorage
// unten, die erst nach einem Nutzer-Entscheid entstehen.
function isBaseSupported(): boolean {
return Boolean(process.env.NEXT_PUBLIC_VAPID_PUBLIC_KEY) && isPushSupported();
}
// Anders als in InstallPrompt.tsx gibt es hier ein echtes externes Signal,
// auf das reagiert werden muss: computeBaseEligible() hängt (über
// isInstallPromptPending()) vom Install-Prompt-Status ab, der sich NACH
// unserem Mount ändern kann (Nutzer klickt ihn erst später weg) - INSTALL_
// PROMPT_SETTLED_EVENT signalisiert genau das, useSyncExternalStore ruft dann
// automatisch computeBaseEligible() erneut auf.
function subscribeToInstallPromptSettled(onChange: () => void) {
window.addEventListener(INSTALL_PROMPT_SETTLED_EVENT, onChange);
return () => window.removeEventListener(INSTALL_PROMPT_SETTLED_EVENT, onChange);
}
/**
* Ob der Prompt (erneut) angezeigt werden soll. Nur eine explizite
* Ablehnung (Permission "denied") oder ein manuelles Wegklicken persistieren
* dauerhaft in localStorage - ein technischer Fehler nach erteilter
* Permission (Subscribe/Server) darf den In-App-Retry-Pfad nicht
* blockieren, sonst bliebe der Nutzer ohne Push, bis er Site-Daten
* zurücksetzt (siehe handleEnable). Deckt NICHT den Fall ab, dass die
* Permission bereits erteilt ist, aber die Server-Registrierung fehlte -
* das kann nur async per pushManager.getSubscription() geprüft werden,
* siehe missingServerRegistration-State in NotificationPrompt.
*/
function computeBaseEligible(): boolean {
if (typeof window === "undefined") return false;
if (!process.env.NEXT_PUBLIC_VAPID_PUBLIC_KEY) return false;
if (!isPushSupported()) return false;
if (Notification.permission !== "default") return false; // schon erlaubt oder blockiert
if (!isBaseSupported()) return false;
if (Notification.permission === "denied") return false;
// Wartet, bis InstallPrompt nicht mehr sichtbar ist (installiert oder
// weggeklickt) - sonst überlappen sich beide "fixed bottom-4"-Banner.
if (isInstallPromptPending()) return false;
@ -44,28 +69,55 @@ function computeEligible(): boolean {
}
export function NotificationPrompt() {
const [eligible, setEligible] = useState(computeEligible);
// useSyncExternalStore statt Lazy-useState/Effect (siehe InstallPrompt.tsx):
// getServerSnapshot liefert server- und im allerersten Client-Pass "false",
// kein Hydration-Mismatch, kein setState im Effect-Body nötig.
const baseEligible = useSyncExternalStore(
subscribeToInstallPromptSettled,
computeBaseEligible,
() => false
);
// true, wenn Permission bereits erteilt ist, aber der Browser lokal keine
// Subscription (mehr) hat - dann war die vorherige Registrierung vermutlich
// unterbrochen (siehe handleEnable-Kommentar), der Prompt bleibt trotz
// gesetztem Dismissed-Flag sichtbar, damit ein Retry möglich ist. Kann nicht
// Teil von computeBaseEligible/useSyncExternalStore sein, weil
// getSubscription() ein Promise zurückgibt (kein synchroner Snapshot).
const [missingServerRegistration, setMissingServerRegistration] = useState(false);
const [dismissed, setDismissed] = useState(false);
const [isSubscribing, setIsSubscribing] = useState(false);
const eligible = (baseEligible || missingServerRegistration) && !dismissed;
// Reagiert auf InstallPrompt, das erst nach unserem Mount weggeklickt wird
// (siehe computeEligible) - ansonsten bliebe eligible dauerhaft false.
useEffect(() => {
const handleSettled = () => setEligible(computeEligible());
window.addEventListener(INSTALL_PROMPT_SETTLED_EVENT, handleSettled);
return () => window.removeEventListener(INSTALL_PROMPT_SETTLED_EVENT, handleSettled);
if (isBaseSupported() && Notification.permission === "granted") {
navigator.serviceWorker.ready
.then((registration) => registration.pushManager.getSubscription())
.then((subscription) => {
if (!subscription) setMissingServerRegistration(true);
})
.catch(() => {});
}
}, []);
// Schließen-Button/explizite Ablehnung: dauerhaft in localStorage merken.
const dismiss = () => {
localStorage.setItem(DISMISSED_KEY, "1");
setEligible(false);
setDismissed(true);
};
// Technischer Fehler nach erteilter Permission: NUR den Prompt ausblenden,
// ohne localStorage zu setzen, damit der Nutzer es beim nächsten Laden
// erneut versuchen kann.
const hideWithoutPersisting = () => setDismissed(true);
const handleEnable = async () => {
setIsSubscribing(true);
try {
const permission = await Notification.requestPermission();
if (permission !== "granted") {
dismiss();
// "denied" wird schon von computeBaseEligible() dauerhaft
// ausgeschlossen, kein zusätzliches localStorage-Flag nötig.
hideWithoutPersisting();
return;
}
@ -87,7 +139,10 @@ export function NotificationPrompt() {
if (!result.success) {
// Serverseitig nicht gespeichert - Subscription auf dem Gerät nicht
// verwaist zurücklassen, sonst denkt der Browser dauerhaft, er sei
// angemeldet, ohne dass Push je zugestellt werden könnte.
// angemeldet, ohne dass Push je zugestellt werden könnte. Best-effort:
// schlägt das fehl, bleibt eine verwaiste Browser-Subscription zurück,
// die der useEffect-Check oben beim nächsten Laden über
// getSubscription() erkennt und den Prompt trotzdem wieder anbietet.
await subscription.unsubscribe().catch(() => {});
throw new Error(result.error);
}
@ -95,7 +150,7 @@ export function NotificationPrompt() {
dismiss();
} catch (error) {
console.error("Push-Aktivierung fehlgeschlagen:", error);
dismiss();
hideWithoutPersisting();
} finally {
setIsSubscribing(false);
}

View file

@ -43,7 +43,11 @@ export async function sendPushToRecipient(
endpoint: sub.endpoint,
keys: { p256dh: sub.p256dh, auth: sub.auth },
},
body
body,
// web-push reicht timeout nur weiter, wenn es explizit gesetzt ist -
// ohne das kann ein Endpoint, der die Verbindung annimmt aber nie
// antwortet, das Promise unbegrenzt offen halten.
{ timeout: 10_000 }
)
)
);

View file

@ -11,6 +11,31 @@ export interface PushSubscriptionInput {
};
}
// Bekannte Push-Dienst-Origins der unterstützten Browser. Ohne diese
// Allowlist könnte ein Aufrufer eine beliebige interne HTTPS-URL als
// "Endpoint" einreichen - sendPushToRecipient würde später serverseitig
// Requests dorthin schicken (SSRF). endpoint kommt vom Client und wird nie
// vom Browser selbst gegen eine feste Liste geprüft, daher hier serverseitig.
const ALLOWED_PUSH_ORIGINS = new Set([
"https://fcm.googleapis.com", // Chrome, Edge, andere Chromium-Browser
"https://updates.push.services.mozilla.com", // Firefox
]);
function isAllowedPushEndpoint(endpoint: string): boolean {
let url: URL;
try {
url = new URL(endpoint);
} catch {
return false;
}
if (url.protocol !== "https:") return false;
if (ALLOWED_PUSH_ORIGINS.has(url.origin)) return true;
// Windows/Edge-Legacy-Endpunkte laufen über wechselnde Subdomains
// (wns2-<region>.notify.windows.com), daher Suffix- statt Origin-Check.
return url.hostname.endsWith(".notify.windows.com");
}
/**
* Speichert (oder aktualisiert) die Push-Subscription des eingeloggten
* Nutzers. Gilt für alle Rollen (User, Manager, Employee) - anders als das
@ -32,6 +57,10 @@ export async function subscribeToPush(
return { success: false, error: "Nicht angemeldet." };
}
if (!isAllowedPushEndpoint(input.endpoint)) {
return { success: false, error: "Ungültiger Push-Endpoint." };
}
try {
await prisma.pushSubscription.upsert({
where: { endpoint: input.endpoint },
@ -59,15 +88,23 @@ export async function subscribeToPush(
/**
* Entfernt eine Push-Subscription (z.B. wenn der Nutzer Benachrichtigungen
* im Browser deaktiviert). Kein Auth-Check auf recipientId nötig - der
* Endpoint ist ein unrateables, gerätespezifisches Secret, das der Client
* nur kennt, wenn er es selbst erzeugt hat.
* im Browser deaktiviert). Auf recipientId des eingeloggten Nutzers
* beschränkt, damit niemand mit einem fremden (z.B. erratenen oder aus
* einem Netzwerk-Log kopierten) Endpoint die Push-Zustellung eines anderen
* Geräts abschalten kann.
*/
export async function unsubscribeFromPush(
endpoint: string
): Promise<{ success: true } | { success: false; error: string }> {
let userData;
try {
await prisma.pushSubscription.deleteMany({ where: { endpoint } });
userData = await requireAuthWithData();
} catch {
return { success: false, error: "Nicht angemeldet." };
}
try {
await prisma.pushSubscription.deleteMany({ where: { endpoint, recipientId: userData.id } });
return { success: true };
} catch (error) {
console.error("unsubscribeFromPush fehlgeschlagen:", error);

View file

@ -14,6 +14,7 @@
* Push aktiviert hat, bekommt dieselben Typen wie in-app).
*/
import { after } from "next/server";
import { prisma } from "@/src/lib/prisma";
import { sendPushToRecipient } from "@/src/lib/push/send-push";
import {
@ -68,20 +69,35 @@ export async function createNotification(input: CreateNotificationInput): Promis
linkUrl: input.linkUrl ?? null,
},
});
// Fire-and-forget: Push-Fehler dürfen das erfolgreiche In-App-Insert
// oben nicht rückgängig machen oder den Aufrufer blockieren.
void sendPushToRecipient(input.recipientId, {
title: input.title,
body: input.body,
url: input.linkUrl,
});
return true;
} catch (error) {
console.error("createNotification fehlgeschlagen:", error);
return false;
}
// Eigener try/catch AUSSERHALB des obigen Blocks: after() wirft, wenn es
// außerhalb eines Request-Scopes aufgerufen wird (z.B. in Tests) - das darf
// den bereits erfolgreichen Insert oben nicht nachträglich als
// fehlgeschlagen erscheinen lassen (return false), sonst denkt der
// Aufrufer, es sei kein Datensatz geschrieben worden, obwohl er es wurde.
try {
// Läuft erst NACH dem Response-Flush weiter, blockiert den Aufrufer also
// nicht - aber die Serverless-Function bleibt bis zum Abschluss aktiv
// (anders als ein reines void-Fire-and-forget, das eine Vercel-Function
// vor Abschluss eingefroren zurücklassen kann). Push-Fehler dürfen das
// In-App-Insert ohnehin nicht mehr rückgängig machen -
// sendPushToRecipient wirft selbst nie.
after(() =>
sendPushToRecipient(input.recipientId, {
title: input.title,
body: input.body,
url: input.linkUrl,
})
);
} catch (error) {
console.error("createNotification: Push-Versand konnte nicht eingeplant werden:", error);
}
return true;
}
/**

View file

@ -270,6 +270,7 @@ const DiscountCodeList = ({
setIsCreating(false);
setForm(emptyForm);
}}
disabled={isSaving}
title="Schließen"
>
<X size={15} />
@ -415,6 +416,7 @@ const DiscountCodeList = ({
setIsCreating(false);
setForm(emptyForm);
}}
disabled={isSaving}
>
Abbrechen
</Button>
@ -507,6 +509,7 @@ const DiscountCodeList = ({
size="icon"
className="size-6 text-blue-400 hover:bg-blue-100/70 hover:text-blue-700"
onClick={cancelEditing}
disabled={isSaving}
title="Schließen"
>
<X size={13} />
@ -590,6 +593,7 @@ const DiscountCodeList = ({
variant="outline"
className="bg-white/60"
onClick={cancelEditing}
disabled={isSaving}
>
Abbrechen
</Button>