diff --git a/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts b/src/Frontend/src/components/configuration/HealthCheckNotifications.spec.ts index ebe697255..494970b37 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 a79564e7f..5a8567a9d 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 2e7a41050..1e9f8c4c6 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 000000000..496fd17e6 --- /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 6aff63c4c..b54d0c334 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);