From 0ceb1fbd518c77cc08d69d20b3c08286941b28f8 Mon Sep 17 00:00:00 2001 From: Florent Clairambault Date: Fri, 13 Nov 2026 10:35:04 +0100 Subject: [PATCH] fix(dash0): save an email check's sender-authentication options on edit The edit form dropped config from every passive check's PATCH, so the SPF, DKIM, DMARC and blocklist options showed on edit but never saved. Email now sends its config; the server keeps the stored inbound token. Co-Authored-By: Claude Opus 5.5 --- .../handlers/checks/email_auth_patch_test.go | 65 ++++++++++++++++++ web/dash0/e2e/email-check-sender-auth.spec.ts | 68 +++++++++++++++++++ .../src/components/shared/check-form.tsx | 9 ++- 3 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 server/internal/handlers/checks/email_auth_patch_test.go diff --git a/server/internal/handlers/checks/email_auth_patch_test.go b/server/internal/handlers/checks/email_auth_patch_test.go new file mode 100644 index 000000000..964ab76ed --- /dev/null +++ b/server/internal/handlers/checks/email_auth_patch_test.go @@ -0,0 +1,65 @@ +package checks_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/fclairamb/solidping/server/internal/handlers/checks" +) + +// TestPatchingEmailAuthOptionsKeepsTheToken covers the dashboard's edit of an +// email check's sender-authentication options (spec 2026-11-12-03): the PATCH +// carries only the options, the inbound address token must survive it, and an +// empty config clears the options again. +func TestPatchingEmailAuthOptionsKeepsTheToken(t *testing.T) { + t.Parallel() + + r := require.New(t) + ctx := t.Context() + svc, dbSvc, _, org := setupPlaintextChecksService(t) + + created, err := svc.CreateCheck(ctx, org.Slug, checks.CreateCheckRequest{ + Name: "email-auth-patch", + Slug: "email-auth-patch", + Type: "email", + Config: map[string]any{}, + }) + r.NoError(err) + + token, ok := created.Config["token"].(string) + r.True(ok) + r.NotEmpty(token) + + patch := map[string]any{ + "require_spf": "pass", + "require_dkim": true, + "require_dmarc": "pass", + "check_blocklists": true, + "blocklists": []any{"zen.spamhaus.org"}, + } + _, err = svc.UpdateCheck(ctx, org.Slug, created.UID, &checks.UpdateCheckRequest{Config: &patch}) + r.NoError(err) + + row, err := dbSvc.GetCheck(ctx, org.UID, created.UID) + r.NoError(err) + r.Equal("pass", row.Config["require_spf"]) + r.Equal(true, row.Config["require_dkim"]) + r.Equal("pass", row.Config["require_dmarc"]) + r.Equal(true, row.Config["check_blocklists"]) + r.Equal([]any{"zen.spamhaus.org"}, row.Config["blocklists"]) + r.Equal(token, row.Config["token"], "the inbound token must survive a PATCH that never mentions it") + + empty := map[string]any{} + _, err = svc.UpdateCheck(ctx, org.Slug, created.UID, &checks.UpdateCheckRequest{Config: &empty}) + r.NoError(err) + + row, err = dbSvc.GetCheck(ctx, org.UID, created.UID) + r.NoError(err) + + for _, key := range []string{"require_spf", "require_dkim", "require_dmarc", "check_blocklists", "blocklists"} { + r.NotContains(row.Config, key, "an empty config clears %s", key) + } + + r.Equal(token, row.Config["token"]) +} diff --git a/web/dash0/e2e/email-check-sender-auth.spec.ts b/web/dash0/e2e/email-check-sender-auth.spec.ts index 0bd338af1..cde0d765b 100644 --- a/web/dash0/e2e/email-check-sender-auth.spec.ts +++ b/web/dash0/e2e/email-check-sender-auth.spec.ts @@ -75,4 +75,72 @@ test.describe("Email check sender authentication", () => { "zen.spamhaus.org\nbl.spamcop.net", ); }); + // The edit route used to drop `config` from a passive check's PATCH, so these + // options showed on edit but were never saved. + test("edits the authentication options of an existing check", async ({ authenticatedPage }) => { + const page = authenticatedPage; + const token = await getAuthToken(page); + await ensureEmailInboxConfigured(page, token); + const headers = { Authorization: `Bearer ${token}` }; + + const created = await page.request.post(`${API_BASE}/api/v1/orgs/test/checks`, { + headers, + data: { type: "email", name: `E2E Email Auth Edit ${Date.now()}`, config: {} }, + }); + expect(created.status()).toBe(201); + const { uid, config: createdConfig } = await created.json(); + const inboxToken = createdConfig.token; + expect(typeof inboxToken).toBe("string"); + + const readConfig = async () => { + const resp = await page.request.get(`${API_BASE}/api/v1/orgs/test/checks/${uid}`, { headers }); + return (await resp.json()).config; + }; + const save = async () => { + const patched = page.waitForResponse( + (r) => r.request().method() === "PATCH" && r.url().includes(`/checks/${uid}`), + ); + await page.getByTestId("check-submit-button").click(); + expect((await patched).status()).toBe(200); + }; + + // Turn every option on. + await page.goto(`orgs/test/checks/${uid}/edit`); + await page.getByTestId("email-auth-section").click(); + await page.getByTestId("email-require-spf-select").click(); + await page.getByRole("option", { name: "Pass", exact: true }).click(); + await page.getByTestId("email-require-dkim-switch").click(); + await page.getByTestId("email-require-dmarc-select").click(); + await page.getByRole("option", { name: "Pass", exact: true }).click(); + await page.getByTestId("email-check-blocklists-switch").click(); + await page.getByTestId("email-blocklists-input").fill("zen.spamhaus.org"); + await save(); + + let config = await readConfig(); + expect(config.require_spf).toBe("pass"); + expect(config.require_dkim).toBe(true); + expect(config.require_dmarc).toBe("pass"); + expect(config.check_blocklists).toBe(true); + expect(config.blocklists).toEqual(["zen.spamhaus.org"]); + expect(config.token).toBe(inboxToken); + + // A reload of the edit form shows them, and turning them off clears them. + await page.goto(`orgs/test/checks/${uid}/edit`); + await expect(page.getByTestId("email-require-dkim-switch")).toBeChecked(); + await page.getByTestId("email-require-spf-select").click(); + await page.getByRole("option", { name: "Off" }).click(); + await page.getByTestId("email-require-dkim-switch").click(); + await page.getByTestId("email-require-dmarc-select").click(); + await page.getByRole("option", { name: "Off" }).click(); + await page.getByTestId("email-check-blocklists-switch").click(); + await save(); + + config = await readConfig(); + expect(config.require_spf).toBeUndefined(); + expect(config.require_dkim).toBeUndefined(); + expect(config.require_dmarc).toBeUndefined(); + expect(config.check_blocklists).toBeUndefined(); + expect(config.blocklists).toBeUndefined(); + expect(config.token).toBe(inboxToken); + }); }); diff --git a/web/dash0/src/components/shared/check-form.tsx b/web/dash0/src/components/shared/check-form.tsx index 9101a7df6..25174aae6 100644 --- a/web/dash0/src/components/shared/check-form.tsx +++ b/web/dash0/src/components/shared/check-form.tsx @@ -1480,8 +1480,13 @@ export function CheckForm({ escalationPolicyUid: escalationPolicyUid || (mode === "edit" ? "" : undefined), period: isPassiveCheckType(type) ? formatPeriod(periodValue, periodUnit) : period, - // Don't send config for passive edits — the token is managed by the backend - ...(isPassiveCheckType(type) && mode === "edit" ? {} : { config: submittedConfig }), + // Passive edits omit config: heartbeat has no editable key and a + // private location's region is read-only. Email is the exception, its + // sender-authentication options live in config. The server keeps the + // stored token when a PATCH leaves it out. + ...(isPassiveCheckType(type) && type !== "email" && mode === "edit" + ? {} + : { config: submittedConfig }), // Automatic placement sends the count and lets the server choose the // regions; pinned sends the explicit list (spec 2026-09-25-06). When // the picker is hidden (fewer than two eligible regions), a NEW check