Let the email configuration dialog read and save its own settings

src/Frontend/src/components/configuration/HealthCheckNotifications.vue

@@ -5,7 +5,6 @@ import ServiceControlAvailable from "../ServiceControlAvailable.vue";
 import HealthCheckNotifications_EmailConfiguration from "./HealthCheckNotifications_ConfigureEmail.vue";
 import { useShowToast } from "@/composables/toast";
 import { TYPE } from "vue-toastification";
-import type UpdateEmailNotificationsSettingsRequest from "@/resources/UpdateEmailNotificationsSettingsRequest";
 import OnOffSwitch from "../OnOffSwitch.vue";
 import FAIcon from "@/components/FAIcon.vue";
 import ActionButton from "@/components/ActionButton.vue";
@@ -15,7 +14,7 @@ import { useHealthChecksStore } from "@/stores/HealthChecksStore";
 import { storeToRefs } from "pinia";
 
 const healthChecksStore = useHealthChecksStore();
-const { emailNotifications, canManageNotifications, canTestNotifications } = storeToRefs(healthChecksStore);
+const { emailNotificationsEnabled, canManageNotifications, canTestNotifications } = storeToRefs(healthChecksStore);
 
 const manageDeniedTooltip = "You don't have permission to manage notifications.";
 const testDeniedTooltip = "You don't have permission to send test notifications.";
@@ -32,19 +31,15 @@ async function toggleEmailNotifications() {
   emailToggleSuccessful.value = await healthChecksStore.toggleEmailNotifications();
 }
 
-async function editEmailNotifications() {
+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;
 }
 
-async function saveEditedEmailNotifications(newSettings: UpdateEmailNotificationsSettingsRequest) {
-  emailUpdateSuccessful.value = null;
+function emailNotificationsSaved(saveSuccessful: boolean) {
   showEmailConfiguration.value = false;
-  const saveSuccessful = await healthChecksStore.saveEmailNotifications(newSettings);
   if (saveSuccessful) {
     emailUpdateSuccessful.value = true;
     useShowToast(TYPE.INFO, "Info", "Email settings updated.");
@@ -84,7 +79,7 @@ onMounted(async () => {
                   <div class="row">
                     <div class="col-auto">
                       <PermissionGate :allowed="canManageNotifications" :reason="manageDeniedTooltip">
-                        <OnOffSwitch id="emailNotifications" @toggle="toggleEmailNotifications" :value="emailNotifications.enabled" :disabled="!canManageNotifications" />
+                        <OnOffSwitch id="emailNotifications" @toggle="toggleEmailNotifications" :value="emailNotificationsEnabled" :disabled="!canManageNotifications" />
                       </PermissionGate>
                       <div>
                         <span class="connection-test connection-failed">
@@ -139,8 +134,8 @@ onMounted(async () => {
       </LicenseNotExpired>
 
       <Teleport to="#modalDisplay">
-        <!-- use the modal component, pass in the prop -->
-        <HealthCheckNotifications_EmailConfiguration v-if="showEmailConfiguration" v-bind="emailNotifications" @cancel="showEmailConfiguration = false" @save="saveEditedEmailNotifications"> </HealthCheckNotifications_EmailConfiguration>
+        <!-- The dialog reads the settings from ServiceControl when it opens, and saves them itself -->
+        <HealthCheckNotifications_EmailConfiguration v-if="showEmailConfiguration" @cancel="showEmailConfiguration = false" @saved="emailNotificationsSaved"> </HealthCheckNotifications_EmailConfiguration>
       </Teleport>
     </ServiceControlAvailable>
   </section>

src/Frontend/src/components/configuration/HealthCheckNotifications_ConfigureEmail.vue

@@ -1,25 +1,57 @@
 <script setup lang="ts">
-import { computed, ref } from "vue";
-import type EmailSettings from "@/components/configuration/EmailSettings";
+import { computed, onMounted, ref } from "vue";
+import serviceControlClient from "@/components/serviceControlClient";
+import type EmailNotifications from "@/resources/EmailNotifications";
 import type UpdateEmailNotificationsSettingsRequest from "@/resources/UpdateEmailNotificationsSettingsRequest";
 import ModalDialog from "../ModalDialog.vue";
 import ActionButton from "../ActionButton.vue";
 import ExternalLink from "@/components/ExternalLink.vue";
+import LoadingSpinner from "@/components/LoadingSpinner.vue";
+import logger from "@/logger";
 
+// The dialog reads the settings from ServiceControl when it opens, edits them here, and writes them back when saved.
+// Nothing outlives the dialog: closing it drops everything below.
 const emit = defineEmits<{
-  save: [settings: UpdateEmailNotificationsSettingsRequest];
+  saved: [succeeded: boolean];
   cancel: [];
 }>();
 
-const settings = defineProps<EmailSettings>();
+const loading = ref(true);
+const loadFailed = ref(false);
+const saving = ref(false);
 
-const smtp_server = ref(settings.smtp_server);
-const smtp_port = ref(settings.smtp_port);
-const authentication_account = ref(settings.authentication_account);
-const authentication_password = ref(settings.authentication_password);
-const enable_tls = ref(settings.enable_tls);
-const from = ref(settings.from);
-const to = ref(settings.to);
+const smtp_server = ref("");
+const smtp_port = ref<number | null>(null);
+const authentication_account = ref("");
+// Always starts empty: the stored password is never shown.
+const authentication_password = ref("");
+const enable_tls = ref<boolean | null>(null);
+const from = ref("");
+const to = ref("");
+
+// Backward compatibility with ServiceControl before 6.21.2: those versions return the stored password, and replace it
+// with whatever a save sends. It is kept here, never shown, to send back when the password is left empty.
+// ServiceControl 6.21.2 and later (https://github.com/Particular/ServiceControl/pull/5980) never return it, so this
+// stays empty, and they keep the stored password themselves when an empty one is sent.
+let storedPassword = "";
+
+onMounted(async () => {
+  try {
+    const [, settings] = await serviceControlClient.fetchTypedFromServiceControl<EmailNotifications>("notifications/email");
+    smtp_server.value = settings.smtp_server ?? "";
+    smtp_port.value = settings.smtp_port || null;
+    authentication_account.value = settings.authentication_account ?? "";
+    enable_tls.value = settings.enable_tls;
+    from.value = settings.from ?? "";
+    to.value = settings.to ?? "";
+    // Only ServiceControl before 6.21.2 returns the password (backward compatibility, see storedPassword).
+    storedPassword = settings.authentication_password ?? "";
+  } catch (err) {
+    logger.error(err);
+    loadFailed.value = true;
+  }
+  loading.value = false;
+});
 
 const emailRe = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
 
@@ -44,17 +76,38 @@ function validateMultipleEmailsCommaSeparated(value: string) {
   return result.every((address) => emailRe.test(address));
 }
 
-function save() {
-  const updatedSettings: UpdateEmailNotificationsSettingsRequest = {
+async function save() {
+  const settings: UpdateEmailNotificationsSettingsRequest = {
     smtp_server: smtp_server.value,
     smtp_port: smtp_port.value ?? 0,
     authorization_account: authentication_account.value,
-    authorization_password: authentication_password.value,
+    authorization_password: passwordToSave(),
     enable_tls: enable_tls.value ?? false,
     from: from.value,
     to: to.value,
   };
-  emit("save", updatedSettings);
+  saving.value = true;
+  try {
+    const response = await serviceControlClient.postToServiceControl("notifications/email", settings);
+    if (!response.ok) logger.error(`Saving the email settings failed: ${response.statusText}`);
+    emit("saved", response.ok);
+  } catch (err) {
+    logger.error(err);
+    emit("saved", false);
+  } finally {
+    saving.value = false;
+  }
+}
+
+function passwordToSave() {
+  // A new password replaces the stored one.
+  if (authentication_password.value) return authentication_password.value;
+  // Without an account, the password is never used, so it is removed.
+  if (!authentication_account.value) return "";
+  // Otherwise keep the stored password. For backward compatibility with ServiceControl before 6.21.2, which would erase
+  // it, it is sent back. ServiceControl 6.21.2 and later (https://github.com/Particular/ServiceControl/pull/5980) keep
+  // it themselves when an empty password is sent, and storedPassword is empty there.
+  return storedPassword;
 }
 
 function close() {
@@ -65,7 +118,9 @@ function close() {
 <template>
   <ModalDialog id="email-config-dialog" title="Email configuration" @close="close" width="800px">
     <template #modalBody>
-      <div class="notificationsForm">
+      <LoadingSpinner v-if="loading" />
+      <div v-else-if="loadFailed" class="alert alert-danger" role="alert">The email settings could not be loaded from ServiceControl. Close this dialog and try again.</div>
+      <div v-else class="notificationsForm">
         <div class="row">
           <div class="form-group" :class="{ 'has-error': !smtpServerIsValid }">
             <label for="smtpServerAddress">SMTP server address</label>
@@ -111,7 +166,7 @@ function close() {
       </div>
     </template>
     <template #modalFooter>
-      <ActionButton variant="primary" :disabled="!formIsValid" @click="save">Save</ActionButton>
+      <ActionButton variant="primary" :disabled="loading || loadFailed || saving || !formIsValid" @click="save">Save</ActionButton>
       <ActionButton @click="close">Cancel</ActionButton>
     </template>
   </ModalDialog>

src/Frontend/src/resources/EmailNotifications.ts

@@ -3,6 +3,7 @@ export default interface EmailNotifications {
   smtp_server?: string;
   smtp_port?: number;
   authentication_account?: string;
+  // Only returned by ServiceControl versions before 6.21.2.
   authentication_password?: string;
   enable_tls: boolean;
   to?: string;

src/Frontend/src/stores/HealthChecksStore.spec.ts

@@ -2,6 +2,9 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
 import { createPinia, setActivePinia } from "pinia";
 import serviceControlClient from "@/components/serviceControlClient";
 import { useHealthChecksStore } from "@/stores/HealthChecksStore";
+import type EmailNotifications from "@/resources/EmailNotifications";
+
+const storedPassword = "stored-secret";
 
 beforeEach(() => {
   setActivePinia(createPinia());
@@ -21,12 +24,12 @@ describe("FEATURE: Turning email notifications on and off", () => {
        * And they are shown as on
        */
       const store = await storeWithEmailNotifications({ enabled: false });
-      const toggle = toggleSucceeds();
+      const toggle = postSucceeds();
 
       await store.toggleEmailNotifications();
 
       expect(toggle).toHaveBeenCalledWith("notifications/email/toggle", { enabled: true });
-      expect(store.emailNotifications.enabled).toBe(true);
+      expect(store.emailNotificationsEnabled).toBe(true);
     });
 
     test("EXAMPLE: The one where email notifications are switched on and then off again without reloading, and they are turned off", async () => {
@@ -38,14 +41,14 @@ describe("FEATURE: Turning email notifications on and off", () => {
        * And they are shown as off
        */
       const store = await storeWithEmailNotifications({ enabled: false });
-      const toggle = toggleSucceeds();
+      const toggle = postSucceeds();
 
       await store.toggleEmailNotifications();
       await store.toggleEmailNotifications();
 
       expect(toggle).toHaveBeenNthCalledWith(1, "notifications/email/toggle", { enabled: true });
       expect(toggle).toHaveBeenNthCalledWith(2, "notifications/email/toggle", { enabled: false });
-      expect(store.emailNotifications.enabled).toBe(false);
+      expect(store.emailNotificationsEnabled).toBe(false);
     });
 
     test("COUNTER-EXAMPLE: The one where ServiceControl rejects the change, and they are shown as they were", async () => {
@@ -61,28 +64,28 @@ describe("FEATURE: Turning email notifications on and off", () => {
       const toggled = await store.toggleEmailNotifications();
 
       expect(toggled).toBe(false);
-      expect(store.emailNotifications.enabled).toBe(false);
+      expect(store.emailNotificationsEnabled).toBe(false);
     });
   });
 });
 
-describe("HealthChecksStore", () => {
-  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",
-    });
+describe("FEATURE: The stored SMTP password is write-only", () => {
+  describe("RULE: Must not hold on to the SMTP password", () => {
+    test("EXAMPLE: The one where ServiceControl older than 6.21.2 returns the stored password when the page loads, and it is not kept", async () => {
+      /*
+       * Given ServiceControl older than 6.21.2, which returns the stored password
+       * When the page loads the email notification settings
+       * Then whether notifications are enabled is known
+       * And the password is not kept
+       */
+      vi.spyOn(serviceControlClient, "fetchTypedFromServiceControl").mockResolvedValue([new Response(), settingsWithStoredPassword()]);
+      const store = useHealthChecksStore();
 
-    expect(saved).toBe(true);
-    expect(store.emailNotifications.authentication_password).toBe("");
+      await store.refresh();
+
+      expect(store.emailNotificationsEnabled).toBe(true);
+      expect(JSON.stringify(store.$state)).not.toContain(storedPassword);
+    });
   });
 });
 
@@ -93,6 +96,19 @@ async function storeWithEmailNotifications({ enabled }: { enabled: boolean }) {
   return store;
 }
 
-function toggleSucceeds() {
+function postSucceeds() {
   return vi.spyOn(serviceControlClient, "postToServiceControl").mockResolvedValue(new Response(null, { status: 200 }));
 }
+
+function settingsWithStoredPassword(): EmailNotifications {
+  return {
+    enabled: true,
+    enable_tls: true,
+    smtp_server: "smtp.example.com",
+    smtp_port: 587,
+    authentication_account: "user@example.com",
+    authentication_password: storedPassword,
+    from: "from@example.com",
+    to: "to@example.com",
+  };
+}