From fcd6dca8522119e16774091abb1d905f6e1b4673 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Thu, 20 Aug 2026 20:41:33 +0200 Subject: [PATCH] fix(push): "der kommer ikke nogen" kunne ikke besvares MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Alt om det var usynligt. Indstillingssiden sagde "til for denne enhed", så snart der fandtes et abonnement — også når hver eneste push blev afvist. Og en afvist push gik i en logline på **Debug**, som ikke skrives på en instans, der kører på standardniveau. Så en levering, der fejler lydløst, og en levering, der aldrig sker, så ens ud, og forskellen er hele diagnosen. Tre forskellige problemer havde ét udseende: browseren tilmeldte sig aldrig, tjenesten afviste den, eller der var ikke noget forfaldent endnu. **En prøveknap** under Indstillinger → Notifikationer sender én notifikation gennem den rigtige rute — ikke en genvej, så det, den beviser, er det, der faktisk skal virke — og siger hvad hver enhed gjorde ved den. Kom den ikke frem, og afviste ingen den, står der, at det så er browseren eller styresystemet, der holder den tilbage. Nu peger svaret på ét af de tre steder. Loglinjen er hævet fra Debug til Warn. Det var den ene linje, der ville have sagt hvorfor. Teksten kommer fra klienten, fordi ordbøgerne gør. En server, der bærer sin egen kopi af "det her er en prøve", bærer den på ét sprog. **Og en lækage, prøven fandt.** Go pakker en transportfejl som `*url.Error`, og dens tekst er `Post "https://fcm.googleapis.com/fcm/send/dQw4…": dial tcp: …` — hele endepunktet, som er den legitimation, push-tjenesten udstedte til den enhed. Den var på vej ud i svaret, i loglinjen og i ethvert skærmbillede af nogen af dem. Prøven var skrevet for at bevise, at kun værten blev nævnt, og det var sådan den blev fundet. Fejlen pakkes ud, og adressen skrubbes bagefter alligevel: den næste fejltype, der bærer den med, kommer ikke med en advarsel. Co-Authored-By: Claude Opus 5 --- docs/openapi.yaml | 47 +++++++ internal/httpapi/integrations_test.go | 61 +++++++++ internal/httpapi/notifications_handlers.go | 124 +++++++++++++++++- internal/httpapi/server.go | 3 + web/src/lib/api.js | 2 + web/src/lib/locales/da.js | 7 + web/src/lib/locales/en.js | 7 + .../indstillinger/notifikationer/+page.svelte | 52 ++++++++ 8 files changed, 299 insertions(+), 4 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 75f03a1..ad081fd 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2079,6 +2079,53 @@ paths: "404": { $ref: "#/components/responses/NotFound" } "422": { $ref: "#/components/responses/Validation" } + /api/v1/push/test: + post: + tags: [notifications] + summary: Send one notification to your own devices, and say what happened. + description: | + The answer to "no notifications arrive". A push that a service refuses is + the ordinary failure — a stale subscription, a browser that revoked + permission without telling the server — and every one of them looks + exactly like silence from the outside. + + It goes through the same path a real notification takes, so what it proves + is the thing that has to work. The title and body come from the caller + because the dictionaries live in the interface. + requestBody: + required: false + content: + application/json: + schema: + type: object + properties: + title: { type: string, maxLength: 200 } + body: { type: string, maxLength: 500 } + responses: + "200": + description: What each device did with it. + content: + application/json: + schema: + type: object + properties: + subscriptions: + type: integer + description: Devices registered for this account. + sent: + type: integer + failed: + type: array + items: + type: object + properties: + service: + type: string + description: The push service's host — never the endpoint, which is a credential. + reason: + type: string + description: What the service said. + /api/v1/sections/{sectionID}: parameters: - { name: sectionID, in: path, required: true, schema: { type: string } } diff --git a/internal/httpapi/integrations_test.go b/internal/httpapi/integrations_test.go index 5533271..3ec1893 100644 --- a/internal/httpapi/integrations_test.go +++ b/internal/httpapi/integrations_test.go @@ -2,6 +2,7 @@ package httpapi import ( "encoding/json" + "fmt" "io" "net/http" "net/url" @@ -9,6 +10,7 @@ import ( "testing" "github.com/kristianwind/verdande/internal/config" + "github.com/kristianwind/verdande/internal/store" ) // --- MCP ----------------------------------------------------------------------------- @@ -1468,3 +1470,62 @@ func TestTheMailAddressCanBeRotated(t *testing.T) { t.Error("the second rotation returned the same address") } } + +// "Der kommer ingen notifikationer" må kunne besvares. +// +// Alt om det var usynligt: indstillingssiden sagde "til for denne enhed", så snart +// der fandtes et abonnement, og en push, tjenesten afviste, gik i en Debug-linje, +// som ikke skrives på en instans, der kører på standardniveau. Man kunne ikke +// skelne "browseren tilmeldte sig aldrig" fra "tjenesten afviste den" fra "der er +// ikke noget forfaldent endnu" — tre forskellige problemer med ét udseende. +func TestATestNotificationSaysWhatHappenedToIt(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + // Ingen enheder: svaret skal sige nul frem for at lade som om det gik godt. + resp, body := ts.do(t, "POST", "/api/v1/push/test", map[string]any{ + "title": "Prøve", "body": "kom den frem?", + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status %d, body %v", resp.StatusCode, body) + } + if body["subscriptions"] != float64(0) { + t.Errorf("subscriptions = %v uden tilmeldte enheder", body["subscriptions"]) + } + + // En enhed, der peger på en tjeneste, som ikke findes. Den skal komme tilbage + // som et navngivent afslag og ikke som tavshed. + user, err := ts.db.UserByEmail(t.Context(), "kristian@example.dk") + if err != nil { + t.Fatal(err) + } + if err := ts.db.SavePushSubscription(t.Context(), &store.PushSubscription{ + UserID: user.ID, + Endpoint: "https://push.example.invalid/abc", + P256dh: "BEl62iUYgUivxIkv69yViEuiBIa-Ib9-SkvMeAtA3LFgDzkrxZJjSgSnfckjBJuBkr3qBUYIHBQFLXYp5Nksh8U", + Auth: "aUZmY2xDS2FQUXJJVmJIeA", + }); err != nil { + t.Fatal(err) + } + + _, body = ts.do(t, "POST", "/api/v1/push/test", map[string]any{"title": "Prøve"}) + if body["subscriptions"] != float64(1) { + t.Fatalf("subscriptions = %v", body["subscriptions"]) + } + failed, _ := body["failed"].([]any) + if len(failed) != 1 { + t.Fatalf("failed har %d poster, want 1 — afslaget blev slugt", len(failed)) + } + first, _ := failed[0].(map[string]any) + if first["service"] != "push.example.invalid" { + t.Errorf("service = %v", first["service"]) + } + if first["reason"] == "" || first["reason"] == nil { + t.Error("afslaget kom uden en grund") + } + // Og aldrig hele adressen: resten af den er den legitimation, tjenesten gav + // enheden, og den hører hverken i et svar, en log eller et skærmbillede. + if strings.Contains(fmt.Sprint(first), "/abc") { + t.Errorf("endepunktet lækkede med ud: %v", first) + } +} diff --git a/internal/httpapi/notifications_handlers.go b/internal/httpapi/notifications_handlers.go index 96d53a4..1161c55 100644 --- a/internal/httpapi/notifications_handlers.go +++ b/internal/httpapi/notifications_handlers.go @@ -1,7 +1,9 @@ package httpapi import ( + "errors" "net/http" + "net/url" "strings" "time" @@ -106,18 +108,44 @@ func (s *Server) PushToUser(userID, title, body, projectID string) { s.pushToUser(userID, title, body, projectID) } -func (s *Server) pushToUser(userID, title, body, projectID string) { +// pushOutcome is what happened, per device. +// +// It exists because "nothing arrives" was unanswerable. A push that a service +// refuses is the ordinary failure here — a stale subscription, a VAPID subject the +// service will not take, a browser that revoked permission without telling the +// server — and every one of them looked exactly like silence. +type pushOutcome struct { + Subscriptions int `json:"subscriptions"` + Sent int `json:"sent"` + Failed []pushFailure `json:"failed"` +} + +// pushFailure names the service and quotes what it said. +// +// The host and not the endpoint: the rest of that URL is the credential the push +// service issued for this device, and it does not belong in an answer, a log line +// or a screenshot. +type pushFailure struct { + Service string `json:"service"` + Reason string `json:"reason"` +} + +func (s *Server) pushToUser(userID, title, body, projectID string) pushOutcome { ctx, cancel := contextWithBackgroundTimeout(30 * time.Second) defer cancel() + var out pushOutcome subs, err := s.db.ListPushSubscriptions(ctx, userID) if err != nil || len(subs) == 0 { - return + return out } + out.Subscriptions = len(subs) + public, private, err := s.vapidKeys(ctx) if err != nil { s.log.Warn("vapid keys", "err", err) - return + out.Failed = append(out.Failed, pushFailure{Service: "verdande", Reason: err.Error()}) + return out } payload := push.Payload{Title: title, Body: body, URL: "/projekt/" + projectID} @@ -128,13 +156,101 @@ func (s *Server) pushToUser(userID, title, body, projectID string) { Public: public, Private: private, Subject: "mailto:" + s.cfg.SMTP.From, }) if err == nil { + out.Sent++ continue } - s.log.Debug("web push failed", "err", err, "endpoint", sub.Endpoint) + // Warn, not Debug. + // + // This was the one line that would have said why no notification ever + // arrived, and at Debug it is not printed on an instance running at the + // default level. A delivery that fails silently and a delivery that does + // not happen look the same from the outside, and the difference is the + // whole diagnosis. + reason := pushReason(sub.Endpoint, err) + s.log.Warn("web push refused", "reason", reason, "service", pushHost(sub.Endpoint), "user", userID) + out.Failed = append(out.Failed, pushFailure{ + Service: pushHost(sub.Endpoint), + Reason: reason, + }) if recErr := s.db.RecordPushFailure(ctx, sub.Endpoint, push.IsGone(err)); recErr != nil { s.log.Warn("record push failure", "err", recErr) } } + return out +} + +// pushReason is what went wrong, with the endpoint taken back out of it. +// +// Go wraps a transport failure as `*url.Error`, and its message is `Post +// "https://fcm.googleapis.com/fcm/send/dQw4w9…": dial tcp: …` — the whole +// endpoint, which is the credential the push service issued for that device. It +// would have gone into the answer, the log line and any screenshot of either. A +// test written to prove the host was reported instead is what found it. +// +// Unwrapped first, so the cause is what is quoted. Then the endpoint is scrubbed +// anyway: the next error type to carry it will not come with a warning. +func pushReason(endpoint string, err error) string { + var urlErr *url.Error + if errors.As(err, &urlErr) && urlErr.Err != nil { + err = urlErr.Err + } + reason := err.Error() + if endpoint != "" { + reason = strings.ReplaceAll(reason, endpoint, pushHost(endpoint)) + } + return reason +} + +// pushHost is the service an endpoint belongs to, and nothing else from it. +func pushHost(endpoint string) string { + u, err := url.Parse(endpoint) + if err != nil || u.Host == "" { + return "ukendt" + } + return u.Host +} + +// handlePushTest sends one notification to this account's devices and says what +// happened to it. +// +// Reported as "der kommer ikke nogen". Everything about that was invisible: the +// settings page said "on for this device" as soon as a subscription existed, and a +// refusal from the push service went into a Debug line nobody sees. There was no +// way to tell "the browser never subscribed" from "the service refused it" from +// "nothing has come due yet" — three different problems with one appearance. +// +// It goes through the same path as a real notification rather than a shortcut, so +// what it proves is the thing that actually has to work. +// +// The words come from the client, which is where the dictionaries are. A server +// that carries its own copy of "This is a test" carries it in one language, and +// this one deliberately keeps its prose in the interface — see +// `internal/i18n`. The text is only ever shown back to the person who asked for +// it, on their own devices. +func (s *Server) handlePushTest(w http.ResponseWriter, r *http.Request) { + var req struct { + Title string `json:"title"` + Body string `json:"body"` + } + if err := decodeJSON(w, r, &req); err != nil { + return + } + title := strings.TrimSpace(req.Title) + if title == "" { + title = "verdande" + } + // Bounded: this is written into a notification on somebody's lock screen, and + // a megabyte of title is not a notification. + if len(title) > 200 { + title = title[:200] + } + body := strings.TrimSpace(req.Body) + if len(body) > 500 { + body = body[:500] + } + + user := userFrom(r.Context()) + writeJSON(w, http.StatusOK, s.pushToUser(user.ID, title, body, "")) } func (s *Server) vapidKeys(ctx contextLike) (string, string, error) { diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index cf74865..5b6af2c 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -295,6 +295,9 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server r.Get("/key", s.handlePushKey) r.Post("/subscribe", s.handleSubscribePush) r.Post("/unsubscribe", s.handleUnsubscribePush) + // One notification, to this account's own devices, reporting what + // each of them did with it. The answer to "der kommer ikke nogen". + r.Post("/test", s.handlePushTest) }) r.Route("/attachments/{attachmentID}", func(r chi.Router) { diff --git a/web/src/lib/api.js b/web/src/lib/api.js index 354611f..695f3c7 100644 --- a/web/src/lib/api.js +++ b/web/src/lib/api.js @@ -366,6 +366,8 @@ export const api = { pushKey: () => get('/push/key'), subscribePush: (subscription) => post('/push/subscribe', subscription), unsubscribePush: (endpoint) => post('/push/unsubscribe', { endpoint, keys: {} }), + // Teksten kommer herfra, fordi ordbøgerne gør. Serveren har ingen. + testPush: (title, body) => post('/push/test', { title, body }), // --- integrations getMailAddress: () => get('/mail-address'), diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index de2d0d2..2489d52 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -242,6 +242,13 @@ export const da = { 'push.turnOff': 'Slå fra', 'push.turnOn': 'Slå til på denne enhed', 'push.perBrowser': 'Gælder kun den browser, du sidder i nu.', + 'push.test': 'Send en prøve', + 'push.testing': 'Sender …', + 'push.testTitle': 'Prøve fra verdande', + 'push.testBody': 'Kom den frem, virker notifikationer på denne enhed.', + 'push.testSent': 'Sendt til {n} enhed(er). Kom den ikke frem, er det browseren eller styresystemet, der holder den tilbage — se om verdande må sende notifikationer.', + 'push.testNone': 'Ingen enheder er tilmeldt endnu. Slå notifikationer til her på enheden først.', + 'push.testFailed': '{service} afviste den: {reason}', 'push.recent': 'Seneste', 'push.none': 'Ingenting endnu.', 'push.open': 'Åbn', diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index cdbdbea..187fe92 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -238,6 +238,13 @@ export const en = { 'push.turnOff': 'Turn off', 'push.turnOn': 'Turn on for this device', 'push.perBrowser': 'Applies only to the browser you are in right now.', + 'push.test': 'Send a test', + 'push.testing': 'Sending …', + 'push.testTitle': 'Test from verdande', + 'push.testBody': 'If this arrived, notifications work on this device.', + 'push.testSent': 'Sent to {n} device(s). If nothing arrived, the browser or the operating system is holding it back — check that verdande is allowed to send notifications.', + 'push.testNone': 'No devices are registered yet. Turn notifications on here first.', + 'push.testFailed': '{service} refused it: {reason}', 'push.recent': 'Recent', 'push.none': 'Nothing yet.', 'push.open': 'Open', diff --git a/web/src/routes/indstillinger/notifikationer/+page.svelte b/web/src/routes/indstillinger/notifikationer/+page.svelte index 05f0c81..5f0e6c7 100644 --- a/web/src/routes/indstillinger/notifikationer/+page.svelte +++ b/web/src/routes/indstillinger/notifikationer/+page.svelte @@ -35,6 +35,43 @@ } } + let testing = $state(false); + let testResult = $state(null); + + /** + * Én notifikation, hele vejen gennem den rigtige rute, og et svar på hvad der + * skete med den. + * + * Den siger ikke "sendt" og lader det være. Kommer den frem, ved man det, fordi + * den ligger på skærmen; kommer den ikke, står der her, hvem der afviste den og + * med hvilke ord — og hvis ingen afviste den, står der, at det så er browseren + * eller styresystemet, der holder den tilbage. Det er de tre steder, den kan + * blive væk, og nu peger svaret på ét af dem. + */ + async function testPush() { + testing = true; + testResult = null; + try { + const r = await api.testPush(t('push.testTitle'), t('push.testBody')); + if (!r.subscriptions) { + testResult = { bad: true, text: t('push.testNone') }; + } else if (r.failed?.length) { + testResult = { + bad: true, + text: r.failed + .map((f) => t('push.testFailed', { service: f.service, reason: f.reason })) + .join(' ') + }; + } else { + testResult = { bad: false, text: t('push.testSent', { n: r.sent }) }; + } + } catch (e) { + testResult = { bad: true, text: humanMessage(e) }; + } finally { + testing = false; + } + } + async function disablePush() { busy = true; try { @@ -165,7 +202,18 @@

{t('push.on')}

+ +
+ {#if testResult} +

{testResult.text}

+ {/if} {:else}