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, diff --git a/crates/defguard_core/src/handlers/auth.rs b/crates/defguard_core/src/handlers/auth.rs index f40552c65..64ba738cf 100644 --- a/crates/defguard_core/src/handlers/auth.rs +++ b/crates/defguard_core/src/handlers/auth.rs @@ -496,6 +496,7 @@ pub async fn webauthn_finish( &mut conn, Some(&session.session.into()), &MFAMethod::Webauthn, + &user.first_name, ) .await?; user.set_mfa_method(&mut *conn, MFAMethod::Webauthn).await?; @@ -658,6 +659,7 @@ pub async fn totp_enable( &mut conn, Some(&session.session.into()), &MFAMethod::OneTimePassword, + &user.first_name, ) .await?; user.set_mfa_method(&mut *conn, MFAMethod::OneTimePassword) @@ -838,6 +840,7 @@ pub async fn email_mfa_enable( &mut conn, Some(&session.session.into()), &MFAMethod::Email, + &user.first_name, ) .await?; user.set_mfa_method(&mut *conn, MFAMethod::Email).await?; 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", 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()) diff --git a/crates/defguard_mail/src/templates.rs b/crates/defguard_mail/src/templates.rs index ae3556ae8..28a147383 100644 --- a/crates/defguard_mail/src/templates.rs +++ b/crates/defguard_mail/src/templates.rs @@ -393,9 +393,11 @@ pub async fn mfa_configured_mail( conn: &mut PgConnection, session: Option<&SessionContext>, method: &MFAMethod, + first_name: &str, ) -> Result<(), TemplateError> { let (mut tera, mut context) = get_base_tera_mjml(Context::new(), session, None, None)?; + context.insert("username", first_name); context.insert("mfa_method", &method); let message = MailMessage::MFAConfigured { method: *method }; diff --git a/crates/defguard_mail/src/tests.rs b/crates/defguard_mail/src/tests.rs index 3eddeed8e..41740d4cb 100644 --- a/crates/defguard_mail/src/tests.rs +++ b/crates/defguard_mail/src/tests.rs @@ -258,6 +258,7 @@ fn send_mfa_configured_mail(_: PgPoolOptions, options: PgConnectOptions) { &mut conn, None, &MFAMethod::Email, + "Test", ) .await .unwrap(); diff --git a/crates/defguard_mail/templates/mfa-configured.text b/crates/defguard_mail/templates/mfa-configured.text index 74dbf9bd3..a9db1f7f9 100644 --- a/crates/defguard_mail/templates/mfa-configured.text +++ b/crates/defguard_mail/templates/mfa-configured.text @@ -1,4 +1,4 @@ -{{ title }} +{{ title }}{% if username %} {{ username }}{% endif %} {% if subtitle %}{{ subtitle }}{% endif %} {{ mfa_method_label }} {{ mfa_method }} diff --git a/crates/defguard_proxy_manager/src/servers/enrollment.rs b/crates/defguard_proxy_manager/src/servers/enrollment.rs index aac2107a6..ce3fbf2d5 100644 --- a/crates/defguard_proxy_manager/src/servers/enrollment.rs +++ b/crates/defguard_proxy_manager/src/servers/enrollment.rs @@ -1074,7 +1074,10 @@ impl EnrollmentServer { .map_err(|_| Status::internal("Failed to get recovery codes.".to_string()))? .ok_or_else(|| Status::internal("Recovery codes not found".to_string()))?; if let Ok(mut conn) = self.pool.begin().await { - if let Err(err) = mfa_configured_mail(&user.email, &mut conn, None, &mfa_method).await { + if let Err(err) = + mfa_configured_mail(&user.email, &mut conn, None, &mfa_method, &user.first_name) + .await + { error!("Failed to send MFA configured email\nReason: {err}"); } } else { diff --git a/web/src/pages/auth/LoginEmail/LoginEmail.tsx b/web/src/pages/auth/LoginEmail/LoginEmail.tsx index c926a19e4..30d8d2f25 100644 --- a/web/src/pages/auth/LoginEmail/LoginEmail.tsx +++ b/web/src/pages/auth/LoginEmail/LoginEmail.tsx @@ -1,10 +1,13 @@ import { useMutation, useQuery } from '@tanstack/react-query'; +import type { AxiosError } from 'axios'; import type z from 'zod'; import { m } from '../../../paraglide/messages'; import api from '../../../shared/api/api'; +import type { ApiError } from '../../../shared/api/types'; import { LoginPage } from '../../../shared/components/LoginPage/LoginPage'; import { SizedBox } from '../../../shared/defguard-ui/components/SizedBox/SizedBox'; import { ThemeSpacing } from '../../../shared/defguard-ui/types'; +import { isPresent } from '../../../shared/defguard-ui/utils/isPresent'; import { useAppForm } from '../../../shared/form'; import { formChangeLogic } from '../../../shared/formLogic'; import { useAuth } from '../../../shared/hooks/useAuth'; @@ -35,6 +38,18 @@ export const LoginEmail = () => { onSuccess: (response) => { useAuth.getState().authSubject.next(response.data); }, + onError: (e: AxiosError) => { + const respCode = e.response?.status; + if (isPresent(respCode) && respCode < 500) { + form.setErrorMap({ + onSubmit: { + fields: { + code: respCode === 429 ? m.login_main_attempts_info() : m.form_error_code(), + }, + }, + }); + } + }, }); const form = useAppForm({ @@ -65,6 +80,7 @@ export const LoginEmail = () => { {(field) => ( { {(field) => ( { {(field) => ( { ), ), 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], ); @@ -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); }, }); 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..b3144e106 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,10 +94,15 @@ const ModalContent = () => { }, }); + const { mutate: resendEmail, isPending: isResending } = useMutation({ + mutationFn: api.auth.mfa.email.init, + }); + const form = useAppForm({ defaultValues, validationLogic: formChangeLogic, validators: { + onMount: formSchema, onSubmit: formSchema, onChange: formSchema, }, @@ -118,6 +123,7 @@ const ModalContent = () => { }); const isSubmitting = useStore(form.store, (s) => s.isSubmitting); + const canSubmit = useStore(form.store, (s) => s.canSubmit); useEffectOnce(() => { void api.auth.mfa.email.init(); @@ -163,6 +169,7 @@ const ModalContent = () => { testId: 'submit', text: m.controls_submit(), loading: isSubmitting, + disabled: !canSubmit || isSubmitting, onClick: () => { form.handleSubmit(); }, @@ -172,7 +179,8 @@ const ModalContent = () => {