From aabe1a1ac67c5f15cb888ca19588ceaac4cab720 Mon Sep 17 00:00:00 2001 From: Ramon Smits Date: Fri, 9 Oct 2026 00:28:45 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Explain=20that=20the=20stored=20SMT?= =?UTF-8?q?P=20password=20is=20never=20shown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ServiceControl no longer returns the stored SMTP password, so the password field in the email configuration is empty when the dialog opens. - The field explains that an empty password keeps the stored one. - autocomplete="new-password" stops the browser from filling in a saved password. - The dialog recommends a dedicated SMTP account with limited send permissions, and links to the documentation. - The dialog loads the settings from ServiceControl when it opens, instead of using the copy loaded with the page. It shows changes made by other users, and the stored password of older ServiceControl versions. - A save no longer copies the settings into the store, so the typed password does not stay in memory. --- .../HealthCheckNotifications.spec.ts | 83 +++++++++++++++++++ .../HealthCheckNotifications.vue | 4 +- ...ealthCheckNotifications_ConfigureEmail.vue | 9 +- .../src/stores/HealthChecksStore.spec.ts | 28 +++++++ src/Frontend/src/stores/HealthChecksStore.ts | 12 +-- 5 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 src/Frontend/src/stores/HealthChecksStore.spec.ts diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts b/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts index ebe6972551..494970b371 100644 --- a/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts +++ b/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts @@ -15,6 +15,7 @@ interface ComponentDSL { discardEmailConfigurationChanges(): Promise; requestTestNotification(): Promise; provideValidEmailConfiguration(): Promise; + settingsChangeInServiceControl(changes: Partial): void; }; verify: { emailNotificationsAreEnabled(): void; @@ -30,6 +31,8 @@ interface ComponentDSL { emailConfigurationWasSaved(): void; emailConfigurationWasNotSaved(): void; emailConfigurationMatches(config: Partial): void; + storedPasswordIsNotShownAndCanBeKept(): void; + limitingTheSmtpAccountIsRecommended(): void; }; } @@ -260,6 +263,70 @@ describe("FEATURE: Health check notifications", () => { }); }); + describe("RULE: The email configuration dialog shows the settings that ServiceControl has now", () => { + test("EXAMPLE: Another user changed the settings after the page was loaded", async () => { + /* + * Given the page was loaded with the To address old@example.com + * And another user changed the To address in ServiceControl to new@example.com + * When the Email configuration popup is opened + * Then the To address is new@example.com + */ + const componentDriver = renderComponent({ + initialState: { + HealthChecksStore: { + emailNotifications: { ...emptyEmailSettings(), to: "old@example.com" }, + }, + }, + }); + + componentDriver.actions.settingsChangeInServiceControl({ to: "new@example.com" }); + await componentDriver.actions.openEmailConfiguration(); + componentDriver.verify.emailConfigurationMatches({ to: "new@example.com" }); + }); + }); + + describe("RULE: The stored SMTP password is never shown", () => { + test("EXAMPLE: ServiceControl has a stored password for the SMTP account", async () => { + /* + * Given ServiceControl has a stored SMTP password + * And ServiceControl does not return it + * When the Email configuration popup is opened + * Then the password field is empty + * And the field explains that an empty password keeps the stored password + */ + const componentDriver = renderComponent({ + initialState: { + HealthChecksStore: { + emailNotifications: { ...emptyEmailSettings(), authentication_account: "user@example.com" }, + }, + }, + }); + + await componentDriver.actions.openEmailConfiguration(); + componentDriver.verify.storedPasswordIsNotShownAndCanBeKept(); + }); + }); + + describe("RULE: The email configuration recommends limiting what the SMTP account can send", () => { + test("EXAMPLE: The Email configuration popup is opened", async () => { + /* + * When the Email configuration popup is opened + * Then it recommends a dedicated SMTP account with limited send permissions + * And it links to the documentation + */ + const componentDriver = renderComponent({ + initialState: { + HealthChecksStore: { + emailNotifications: emptyEmailSettings(), + }, + }, + }); + + await componentDriver.actions.openEmailConfiguration(); + componentDriver.verify.limitingTheSmtpAccountIsRecommended(); + }); + }); + describe("RULE: Sending a test notification indicates whether delivery succeeded or failed", () => { test("EXAMPLE: The email configuration is invalid", async () => { /* @@ -342,6 +409,12 @@ function renderComponent({ initialState = {} }: { initialState?: Record) { + vi.mocked(store.refresh).mockImplementation(() => { + store.emailNotifications = { ...store.emailNotifications, ...changes }; + return Promise.resolve(); + }); + }, async provideValidEmailConfiguration() { await user.type(screen.getByLabelText(/smtp server address/i), "smtp.example.com"); await user.type(screen.getByLabelText(/smtp server port/i), "587"); @@ -386,6 +459,16 @@ function renderComponent({ initialState = {} }: { initialState?: Record) { if (config.smtp_server !== undefined) { expect(screen.getByLabelText(/smtp server address/i)).toHaveValue(config.smtp_server); diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications.vue b/src/Frontend/src/components/configuration/HealthCheckNotifications.vue index a79564e7f4..5a8567a9d4 100644 --- a/src/Frontend/src/components/configuration/HealthCheckNotifications.vue +++ b/src/Frontend/src/components/configuration/HealthCheckNotifications.vue @@ -32,10 +32,12 @@ async function toggleEmailNotifications() { emailToggleSuccessful.value = await healthChecksStore.toggleEmailNotifications(); } -function editEmailNotifications() { +async function editEmailNotifications() { emailToggleSuccessful.value = null; emailTestSuccessful.value = null; emailUpdateSuccessful.value = null; + // Start the form from what ServiceControl has now, not from what was loaded with the page. + await healthChecksStore.refresh(); showEmailConfiguration.value = true; } diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue b/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue index 2e7a410501..1e9f8c4c68 100644 --- a/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue +++ b/src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue @@ -2,6 +2,7 @@ import { computed, ref } from "vue"; import type EmailSettings from "@/components/configuration/EmailSettings"; import type UpdateEmailNotificationsSettingsRequest from "@/resources/UpdateEmailNotificationsSettingsRequest"; +import ExternalLink from "@/components/ExternalLink.vue"; const emit = defineEmits<{ save: [settings: UpdateEmailNotificationsSettingsRequest]; @@ -87,7 +88,13 @@ function close() {
- + +
The stored password is never shown.
+
+
+
+ Use a dedicated SMTP account with limited send permissions. + Securing the SMTP account
diff --git a/src/Frontend/src/stores/HealthChecksStore.spec.ts b/src/Frontend/src/stores/HealthChecksStore.spec.ts new file mode 100644 index 0000000000..496fd17e66 --- /dev/null +++ b/src/Frontend/src/stores/HealthChecksStore.spec.ts @@ -0,0 +1,28 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { createPinia, setActivePinia } from "pinia"; +import serviceControlClient from "@/components/serviceControlClient"; +import { useHealthChecksStore } from "@/stores/HealthChecksStore"; + +describe("HealthChecksStore", () => { + beforeEach(() => { + setActivePinia(createPinia()); + }); + + test("does not keep the SMTP password in memory after the settings are saved", async () => { + vi.spyOn(serviceControlClient, "postToServiceControl").mockResolvedValue(new Response(null, { status: 200 })); + const store = useHealthChecksStore(); + + const saved = await store.saveEmailNotifications({ + smtp_server: "smtp.example.com", + smtp_port: 587, + authorization_account: "user@example.com", + authorization_password: "secret", + enable_tls: true, + from: "from@example.com", + to: "to@example.com", + }); + + expect(saved).toBe(true); + expect(store.emailNotifications.authentication_password).toBe(""); + }); +}); diff --git a/src/Frontend/src/stores/HealthChecksStore.ts b/src/Frontend/src/stores/HealthChecksStore.ts index 6aff63c4c4..b54d0c334d 100644 --- a/src/Frontend/src/stores/HealthChecksStore.ts +++ b/src/Frontend/src/stores/HealthChecksStore.ts @@ -83,16 +83,8 @@ export const useHealthChecksStore = defineStore("HealthChecksStore", () => { async function saveEmailNotifications(newSettings: UpdateEmailNotificationsSettingsRequest) { const result = await getResponseOrError(() => serviceControlClient.postToServiceControl("notifications/email", newSettings)); if (result.message === "success") { - emailNotifications.value = { - enabled: emailNotifications.value.enabled, - enable_tls: newSettings.enable_tls, - smtp_server: newSettings.smtp_server, - smtp_port: newSettings.smtp_port, - authentication_account: newSettings.authorization_account, - authentication_password: newSettings.authorization_password, - from: newSettings.from, - to: newSettings.to, - }; + // The saved settings are not copied into the store. The configuration dialog loads them from + // ServiceControl when it opens, and the password must not stay in memory. return true; } else { logger.error(result.message);