From b4be8011c63bd492c2a785505204e7fcdc5d74e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 08:29:53 +0200 Subject: [PATCH 01/10] make smtp auth form fields nullable --- .../settings/SettingsSmtpPage/SettingsSmtpPage.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx b/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx index 5c8adb381..4e7da8e5e 100644 --- a/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx +++ b/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx @@ -122,8 +122,8 @@ const Content = ({ settings }: { settings: Settings }) => { ), ), smtp_port: z.number(m.form_error_required()).max(65535, m.form_error_port_max()), - smtp_password: z.string().trim(), - smtp_user: z.string().trim(), + smtp_password: z.string().trim().nullable(), + smtp_user: z.string().trim().nullable(), smtp_sender: z .string() .trim() @@ -139,11 +139,11 @@ const Content = ({ settings }: { settings: Settings }) => { const emptyValues = useMemo( (): FormFields => ({ smtp_encryption: SmtpEncryption.StartTls, - smtp_password: '', + smtp_password: null, smtp_port: 587, smtp_sender: '', smtp_server: '', - smtp_user: '', + smtp_user: null, }), [], ); @@ -151,11 +151,11 @@ const Content = ({ settings }: { settings: Settings }) => { const defaultValues = useMemo( (): FormFields => ({ smtp_encryption: settings.smtp_encryption, - smtp_password: settings.smtp_password ?? '', + smtp_password: settings.smtp_password ?? null, smtp_port: settings.smtp_port ?? 587, smtp_sender: settings.smtp_sender ?? '', smtp_server: settings.smtp_server ?? '', - smtp_user: settings.smtp_user ?? '', + smtp_user: settings.smtp_user ?? null, }), [settings], ); From 38acbfa5d0d4ad0bed59f9b4755677708811e52e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 08:42:11 +0200 Subject: [PATCH 02/10] add test to reproduce setting clearing bug --- .../tests/integration/api/settings.rs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/crates/defguard_core/tests/integration/api/settings.rs b/crates/defguard_core/tests/integration/api/settings.rs index a2272be78..86e446fc9 100644 --- a/crates/defguard_core/tests/integration/api/settings.rs +++ b/crates/defguard_core/tests/integration/api/settings.rs @@ -48,6 +48,92 @@ async fn test_settings(_: PgPoolOptions, options: PgConnectOptions) { assert!(new_settings.wireguard_enabled); } +#[sqlx::test] +async fn test_patch_settings_clears_optional_fields(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + let (client, _client_state) = make_test_client(pool.clone()).await; + + let auth = Auth::new("admin", "pass123"); + let response = client.post("/api/v1/auth").json(&auth).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // --- smtp_user & smtp_password --- + + // set smtp_user and smtp_password (include the required trio so validation passes) + let patch: SettingsPatch = serde_json::from_str( + r#"{ + "smtp_server": "smtp.example.com", + "smtp_port": 587, + "smtp_sender": "noreply@example.com", + "smtp_user": "testuser", + "smtp_password": "testpass" + }"#, + ) + .unwrap(); + let response = client.patch("/api/v1/settings").json(&patch).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // verify fields are set + let response = client.get("/api/v1/settings").send().await; + assert_eq!(response.status(), StatusCode::OK); + let settings: Settings = response.json().await; + assert_eq!( + settings.smtp_user, + Some("testuser".to_string()), + "smtp_user should be set after initial PATCH" + ); + // smtp_password is redacted in the API response; verify via DB + let from_db = Settings::get(&pool).await.unwrap().unwrap(); + assert!( + from_db.smtp_password.is_some(), + "smtp_password should be set in DB after initial PATCH" + ); + + // clear smtp_user and smtp_password by sending null + let patch: SettingsPatch = + serde_json::from_str(r#"{ "smtp_user": null, "smtp_password": null }"#).unwrap(); + let response = client.patch("/api/v1/settings").json(&patch).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // assert both fields are cleared in the DB + let from_db = Settings::get(&pool).await.unwrap().unwrap(); + assert!( + from_db.smtp_user.is_none(), + "smtp_user should be cleared to None after PATCH with null" + ); + assert!( + from_db.smtp_password.is_none(), + "smtp_password should be cleared to None after PATCH with null" + ); + + // --- ldap_user_rdn_attr --- + + // set ldap_user_rdn_attr + let patch: SettingsPatch = serde_json::from_str(r#"{ "ldap_user_rdn_attr": "uid" }"#).unwrap(); + let response = client.patch("/api/v1/settings").json(&patch).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // verify field is set + let from_db = Settings::get(&pool).await.unwrap().unwrap(); + assert_eq!( + from_db.ldap_user_rdn_attr, + Some("uid".to_string()), + "ldap_user_rdn_attr should be set after PATCH" + ); + + // clear ldap_user_rdn_attr by sending null + let patch: SettingsPatch = serde_json::from_str(r#"{ "ldap_user_rdn_attr": null }"#).unwrap(); + let response = client.patch("/api/v1/settings").json(&patch).send().await; + assert_eq!(response.status(), StatusCode::OK); + + // assert field is cleared in the DB + let from_db = Settings::get(&pool).await.unwrap().unwrap(); + assert!( + from_db.ldap_user_rdn_attr.is_none(), + "ldap_user_rdn_attr should be cleared to None after PATCH with null" + ); +} + // JSON fragment containing all required LDAP fields except ldap_url (add that at the call site). const VALID_LDAP_FIELDS_NO_URL: &str = r#" "ldap_bind_username": "cn=admin,dc=example,dc=com", From a6cf68b9b27f78703b08fe15028758f4315389aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 08:48:48 +0200 Subject: [PATCH 03/10] add a workaround for optional settings not being cleared --- .../defguard_common/src/db/models/settings.rs | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/defguard_common/src/db/models/settings.rs b/crates/defguard_common/src/db/models/settings.rs index 514c9747e..c9ad8c337 100644 --- a/crates/defguard_common/src/db/models/settings.rs +++ b/crates/defguard_common/src/db/models/settings.rs @@ -10,7 +10,7 @@ use rsa::{ traits::PublicKeyParts, }; use secrecy::ExposeSecret; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use sqlx::{PgExecutor, PgPool, Type, query, query_as}; use struct_patch::Patch; use thiserror::Error; @@ -146,6 +146,27 @@ impl LdapSyncStatus { } } +/// Custom deserializer for `Option` fields in patch structs. +/// +/// By default serde cannot distinguish between a missing JSON key and an explicit `null` value +/// when deserializing into `Option>` (the shape struct-patch generates for nullable +/// fields) — both map to the outer `None`, causing a PATCH with `"field": null` to be silently +/// ignored instead of clearing the field. +/// +/// This deserializer always wraps the result in `Some(...)`, so: +/// - JSON key absent → `None` (handled by `#[serde(default)]` before this is called) +/// - `"field": null` → `Some(None)` → field gets cleared +/// - `"field": "val"` → `Some(Some(v))` → field gets updated +/// +/// See and the struct-patch test suite. +fn deserialize_optional_field<'de, T, D>(deserializer: D) -> Result>, D::Error> +where + D: Deserializer<'de>, + T: Deserialize<'de>, +{ + Ok(Some(Option::deserialize(deserializer)?)) +} + #[derive(Clone, Deserialize, PartialEq, Patch, Serialize, Default)] #[patch(attribute(derive(Deserialize, Serialize, Debug)))] pub struct Settings { @@ -164,7 +185,9 @@ pub struct Settings { pub smtp_server: Option, pub smtp_port: Option, pub smtp_encryption: SmtpEncryption, + #[patch(attribute(serde(deserialize_with = "deserialize_optional_field", default)))] pub smtp_user: Option, + #[patch(attribute(serde(deserialize_with = "deserialize_optional_field", default)))] pub smtp_password: Option, pub smtp_sender: Option, // Enrollment @@ -202,6 +225,7 @@ pub struct Settings { // Additional object classes for users which determine the added attributes pub ldap_user_auxiliary_obj_classes: Vec, // The attribute which is used to map LDAP usernames to Defguard usernames + #[patch(attribute(serde(deserialize_with = "deserialize_optional_field", default)))] pub ldap_user_rdn_attr: Option, pub ldap_sync_groups: Vec, pub ldap_remote_enrollment_enabled: bool, From 9e06efa6a0e1fa7f03dfb11daa9383fcb02d65bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 08:57:26 +0200 Subject: [PATCH 04/10] add missing snackbars --- .../pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx b/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx index 4e7da8e5e..76201c1a5 100644 --- a/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx +++ b/web/src/pages/settings/SettingsSmtpPage/SettingsSmtpPage.tsx @@ -165,6 +165,12 @@ const Content = ({ settings }: { settings: Settings }) => { meta: { invalidate: [['settings'], ['info']], }, + onSuccess: () => { + Snackbar.default(m.settings_msg_saved()); + }, + onError: () => { + Snackbar.error(m.settings_msg_save_failed()); + }, }); const form = useAppForm({ @@ -176,6 +182,7 @@ const Content = ({ settings }: { settings: Settings }) => { }, onSubmit: async ({ value }) => { await editSettings(value); + form.reset(value); }, }); From 4a682ec03333595f1961c05eadd1c0c8078ce2e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 10:07:23 +0200 Subject: [PATCH 05/10] handle smtp user & pass as trulty optional --- crates/defguard_mail/src/lib.rs | 17 ++++++++--------- crates/defguard_mail/src/mail.rs | 6 +++--- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/crates/defguard_mail/src/lib.rs b/crates/defguard_mail/src/lib.rs index 7fe038abd..f14e3888d 100644 --- a/crates/defguard_mail/src/lib.rs +++ b/crates/defguard_mail/src/lib.rs @@ -21,29 +21,28 @@ pub(crate) struct SmtpSettings { server: String, port: u16, encryption: SmtpEncryption, - user: String, - password: String, + user: Option, + password: Option, sender: String, } impl SmtpSettings { /// Constructs `SmtpSettings` from `Settings`. Returns error if `SmtpSettings` are incomplete. pub(crate) fn from_settings(settings: Settings) -> Result { - if let (Some(server), Some(port), encryption, Some(user), Some(password), Some(sender)) = ( + if let (Some(server), Some(port), Some(sender)) = ( settings.smtp_server, settings.smtp_port, - settings.smtp_encryption, - settings.smtp_user, - settings.smtp_password, settings.smtp_sender, ) { let port = port.try_into().map_err(|_| MailError::InvalidPort(port))?; Ok(Self { server, port, - encryption, - user, - password: password.expose_secret().to_string(), + encryption: settings.smtp_encryption, + user: settings.smtp_user, + password: settings + .smtp_password + .map(|p| p.expose_secret().to_string()), sender, }) } else { diff --git a/crates/defguard_mail/src/mail.rs b/crates/defguard_mail/src/mail.rs index 1d4f2d1fe..40e8fd73a 100644 --- a/crates/defguard_mail/src/mail.rs +++ b/crates/defguard_mail/src/mail.rs @@ -260,11 +260,11 @@ impl Mail { .timeout(Some(SMTP_TIMEOUT)); // Skip credentials if any of them is empty - let builder = if settings.user.is_empty() || settings.password.is_empty() { + let builder = if let (Some(user), Some(password)) = (settings.user, settings.password) { + builder.credentials(Credentials::new(user, password)) + } else { debug!("SMTP credentials were not provided, skipping username/password authentication"); builder - } else { - builder.credentials(Credentials::new(settings.user, settings.password)) }; Ok(builder.build()) From 6dc1970d3ac7a6aa87d8903fd8f87ae1a853774f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 28 Apr 2026 10:08:08 +0200 Subject: [PATCH 06/10] make resend button actually resent the email --- .../modals/EmailMfaSetupModal/EmailMfaSetupModal.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/web/src/pages/user-profile/UserProfilePage/tabs/ProfileDetailsTab/modals/EmailMfaSetupModal/EmailMfaSetupModal.tsx b/web/src/pages/user-profile/UserProfilePage/tabs/ProfileDetailsTab/modals/EmailMfaSetupModal/EmailMfaSetupModal.tsx index e726b450a..6390bed3b 100644 --- a/web/src/pages/user-profile/UserProfilePage/tabs/ProfileDetailsTab/modals/EmailMfaSetupModal/EmailMfaSetupModal.tsx +++ b/web/src/pages/user-profile/UserProfilePage/tabs/ProfileDetailsTab/modals/EmailMfaSetupModal/EmailMfaSetupModal.tsx @@ -94,6 +94,10 @@ const ModalContent = () => { }, }); + const { mutate: resendEmail, isPending: isResending } = useMutation({ + mutationFn: api.auth.mfa.email.init, + }); + const form = useAppForm({ defaultValues, validationLogic: formChangeLogic, @@ -172,7 +176,8 @@ const ModalContent = () => {