From f4230421a9837a5cd7ca5d3f95a0ac55fc15b2e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Mon, 16 Mar 2026 16:30:36 +0100 Subject: [PATCH 1/5] add missing disconnect threshold input --- .../EditLocationPage/EditLocationPage.tsx | 198 +++++++++++------- 1 file changed, 126 insertions(+), 72 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 9ff365132e..7d8f0604bc 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -65,38 +65,63 @@ const locationToFirewall = (location: NetworkLocation): LocationFirewallValue => return 'deny'; }; -const formSchema = z.object({ - name: z.string(m.form_error_required()).min(1, m.form_error_required()), - address: z - .string(m.form_error_required()) - .trim() - .min(1, m.form_error_required()) - .refine((value) => validateIpList(value, ',', true), m.form_error_invalid()), - endpoint: z.string(m.form_error_required()).trim().min(1, m.form_error_required()), - port: z.number(m.form_error_required()).max(65535, m.form_error_port_max()), - allowed_ips: z.string(m.form_error_required()).trim(), - dns: z - .string() - .trim() - .nullable() - .refine((val) => { - if (!val) return true; - return validateIpOrDomainList(val, ',', true, true); - }), - peer_disconnect_threshold: z.number(m.form_error_required()), - keepalive_interval: z - .number(m.form_error_required()) - .max(65535, m.form_error_port_max()), - mtu: z.number(m.form_error_required()).min(72).max(0xffffffff), - fwmark: z.number(m.form_error_required()).min(0).max(0xffffffff), - allow_all_groups: z.boolean(), - allowed_groups: z.array( - z.string(m.form_error_required()).trim().min(1, m.form_error_required()), - ), - location_mfa_mode: z.enum(LocationMfaMode), - service_location_mode: z.enum(LocationServiceMode), - firewall: z.enum(LocationFirewall), -}); +const peerDisconnectThresholdMinimum = 120; + +const formSchema = z + .object({ + name: z.string(m.form_error_required()).min(1, m.form_error_required()), + address: z + .string(m.form_error_required()) + .trim() + .min(1, m.form_error_required()) + .refine((value) => validateIpList(value, ',', true), m.form_error_invalid()), + endpoint: z.string(m.form_error_required()).trim().min(1, m.form_error_required()), + port: z.number(m.form_error_required()).max(65535, m.form_error_port_max()), + allowed_ips: z.string(m.form_error_required()).trim(), + dns: z + .string() + .trim() + .nullable() + .refine((val) => { + if (!val) return true; + return validateIpOrDomainList(val, ',', true, true); + }), + peer_disconnect_threshold: z.number().nullable(), + keepalive_interval: z + .number(m.form_error_required()) + .max(65535, m.form_error_port_max()), + mtu: z.number(m.form_error_required()).min(72).max(0xffffffff), + fwmark: z.number(m.form_error_required()).min(0).max(0xffffffff), + allow_all_groups: z.boolean(), + allowed_groups: z.array( + z.string(m.form_error_required()).trim().min(1, m.form_error_required()), + ), + location_mfa_mode: z.enum(LocationMfaMode), + service_location_mode: z.enum(LocationServiceMode), + firewall: z.enum(LocationFirewall), + }) + .superRefine((value, context) => { + if (value.location_mfa_mode === LocationMfaMode.Disabled) { + return; + } + + if (value.peer_disconnect_threshold === null) { + context.addIssue({ + code: 'custom', + path: ['peer_disconnect_threshold'], + message: m.form_error_required(), + }); + return; + } + + if (value.peer_disconnect_threshold < peerDisconnectThresholdMinimum) { + context.addIssue({ + code: 'custom', + path: ['peer_disconnect_threshold'], + message: m.form_min_value({ value: peerDisconnectThresholdMinimum }), + }); + } + }); type FormFields = z.infer; @@ -213,6 +238,10 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { if (clone.location_mfa_mode !== LocationMfaMode.Disabled) { clone.service_location_mode = LocationServiceMode.Disabled; } + + const peerDisconnectThreshold = + clone.peer_disconnect_threshold ?? location.peer_disconnect_threshold; + await editLocation({ id: location.id, data: { @@ -221,6 +250,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { allowed_groups: clone.allowed_groups, acl_default_allow: clone.firewall === LocationFirewall.Allow, acl_enabled: !(clone.firewall === LocationFirewall.Disabled), + peer_disconnect_threshold: peerDisconnectThreshold, }, }); }, @@ -301,60 +331,84 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { s.values.service_location_mode !== LocationServiceMode.Disabled } > - {(disabled) => ( - { - const service = fieldApi.form.getFieldValue('service_location_mode'); - if ( - value !== LocationMfaMode.Disabled && - service !== LocationServiceMode.Disabled - ) { - fieldApi.form.setFieldValue( - 'service_location_mode', - LocationServiceMode.Disabled, - ); - } - }, - }} - > - {(field) => { - return ( - <> - {disabled && ( - - )} - + {(mfaLocked) => ( + <> + {mfaLocked && ( + + )} + + { + const service = fieldApi.form.getFieldValue( + 'service_location_mode', + ); + if ( + value !== LocationMfaMode.Disabled && + service !== LocationServiceMode.Disabled + ) { + fieldApi.form.setFieldValue( + 'service_location_mode', + LocationServiceMode.Disabled, + ); + } + }, + }} + > + {(field) => ( + <> - - - ); - }} - + + )} + + + state.values.location_mfa_mode !== LocationMfaMode.Disabled + } + > + {(showDisconnectThreshold) => + showDisconnectThreshold ? ( + <> + + + {(field) => ( + + )} + + + ) : null + } + + + )} Date: Mon, 16 Mar 2026 16:35:01 +0100 Subject: [PATCH 2/5] rename selectors --- .../EditLocationPage/EditLocationPage.tsx | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/web/src/pages/EditLocationPage/EditLocationPage.tsx b/web/src/pages/EditLocationPage/EditLocationPage.tsx index 7d8f0604bc..1e99bb590c 100644 --- a/web/src/pages/EditLocationPage/EditLocationPage.tsx +++ b/web/src/pages/EditLocationPage/EditLocationPage.tsx @@ -331,9 +331,9 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { s.values.service_location_mode !== LocationServiceMode.Disabled } > - {(mfaLocked) => ( + {(isServiceLocation) => ( <> - {mfaLocked && ( + {isServiceLocation && ( { )} @@ -414,7 +414,7 @@ const EditLocationForm = ({ location }: { location: NetworkLocation }) => { s.values.location_mfa_mode !== LocationMfaMode.Disabled} > - {(disabled) => ( + {(mfaEnabled) => ( { {(field) => { return ( <> - {disabled && ( + {mfaEnabled && ( { From 5dc6ef14228e52bdd49042c31c25bc84a08a829f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Mon, 16 Mar 2026 17:39:34 +0100 Subject: [PATCH 3/5] align backend validation with frontend --- .../defguard_core/src/handlers/wireguard.rs | 18 +++ .../tests/integration/api/wireguard.rs | 118 ++++++++++++++++++ 2 files changed, 136 insertions(+) diff --git a/crates/defguard_core/src/handlers/wireguard.rs b/crates/defguard_core/src/handlers/wireguard.rs index 8a1e48b9e9..9a6bf53d56 100644 --- a/crates/defguard_core/src/handlers/wireguard.rs +++ b/crates/defguard_core/src/handlers/wireguard.rs @@ -73,6 +73,8 @@ pub struct WireguardNetworkData { pub service_location_mode: ServiceLocationMode, } +const MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA: i32 = 120; + impl WireguardNetworkData { pub(crate) fn parse_allowed_ips(&self) -> Vec { self.allowed_ips @@ -103,6 +105,20 @@ impl WireguardNetworkData { Ok(subnets) } + pub(crate) fn validate_peer_disconnect_threshold(&self) -> Result<(), WebError> { + if self.location_mfa_mode == LocationMfaMode::Disabled { + return Ok(()); + } + + if self.peer_disconnect_threshold >= MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA { + return Ok(()); + } + + Err(WebError::BadRequest( + "peer_disconnect_threshold must be at least 120 when location MFA is enabled".into(), + )) + } + pub(crate) async fn validate_location_mfa_mode<'e, E: sqlx::PgExecutor<'e>>( &self, executor: E, @@ -216,6 +232,7 @@ pub(crate) async fn create_network( }); } + data.validate_peer_disconnect_threshold()?; data.validate_location_mfa_mode(&appstate.pool).await?; let allowed_ips = data.parse_allowed_ips(); @@ -324,6 +341,7 @@ pub(crate) async fn modify_network( }); } + data.validate_peer_disconnect_threshold()?; data.validate_location_mfa_mode(&appstate.pool).await?; let mut network = find_network(network_id, &appstate.pool).await?; diff --git a/crates/defguard_core/tests/integration/api/wireguard.rs b/crates/defguard_core/tests/integration/api/wireguard.rs index a36ca92e8a..c1e852df6a 100644 --- a/crates/defguard_core/tests/integration/api/wireguard.rs +++ b/crates/defguard_core/tests/integration/api/wireguard.rs @@ -33,6 +33,9 @@ use super::common::{ authenticate_admin, exceed_enterprise_limits, make_network, make_test_client, setup_pool, }; +const INVALID_MFA_PEER_DISCONNECT_THRESHOLD: i32 = 119; +const MINIMUM_MFA_PEER_DISCONNECT_THRESHOLD: i32 = 120; + #[sqlx::test] async fn test_network(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; @@ -319,6 +322,121 @@ async fn test_location_mfa_mode_validation_modify(_: PgPoolOptions, options: PgC assert_eq!(response.status(), StatusCode::OK); } +#[sqlx::test] +async fn test_peer_disconnect_threshold_validation_create( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + + let (mut client, _client_state) = make_test_client(pool).await; + authenticate_admin(&mut client).await; + + let mut location_data = WireguardNetworkData { + name: "test_location_disabled".into(), + address: "10.1.1.0/24".into(), + endpoint: "10.1.1.1".parse().unwrap(), + port: 55555, + allowed_ips: Some("10.1.1.0/24, 10.2.0.1/16, 10.10.10.54/32".into()), + dns: None, + mtu: DEFAULT_WIREGUARD_MTU, + fwmark: 0, + allow_all_groups: false, + allowed_groups: vec!["admin".into()], + keepalive_interval: DEFAULT_KEEPALIVE_INTERVAL, + peer_disconnect_threshold: INVALID_MFA_PEER_DISCONNECT_THRESHOLD, + acl_enabled: false, + acl_default_allow: false, + location_mfa_mode: LocationMfaMode::Disabled, + service_location_mode: ServiceLocationMode::Disabled, + }; + + let response = client + .post("/api/v1/network") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + + location_data.name = "test_location_internal".into(); + location_data.location_mfa_mode = LocationMfaMode::Internal; + let response = client + .post("/api/v1/network") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + location_data.name = "test_location_internal_boundary".into(); + location_data.peer_disconnect_threshold = MINIMUM_MFA_PEER_DISCONNECT_THRESHOLD; + let response = client + .post("/api/v1/network") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); +} + +#[sqlx::test] +async fn test_peer_disconnect_threshold_validation_modify( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = setup_pool(options).await; + + let (mut client, _client_state) = make_test_client(pool).await; + authenticate_admin(&mut client).await; + + let mut location_data = WireguardNetworkData { + name: "test_location".into(), + address: "10.1.1.0/24".into(), + endpoint: "10.1.1.1".parse().unwrap(), + port: 55555, + allowed_ips: Some("10.1.1.0/24, 10.2.0.1/16, 10.10.10.54/32".into()), + dns: None, + mtu: DEFAULT_WIREGUARD_MTU, + fwmark: 0, + allow_all_groups: false, + allowed_groups: vec!["admin".into()], + keepalive_interval: DEFAULT_KEEPALIVE_INTERVAL, + peer_disconnect_threshold: INVALID_MFA_PEER_DISCONNECT_THRESHOLD, + acl_enabled: false, + acl_default_allow: false, + location_mfa_mode: LocationMfaMode::Disabled, + service_location_mode: ServiceLocationMode::Disabled, + }; + + let response = client + .post("/api/v1/network") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + + let response = client + .put("/api/v1/network/1") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::OK); + + location_data.location_mfa_mode = LocationMfaMode::Internal; + let response = client + .put("/api/v1/network/1") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + location_data.peer_disconnect_threshold = MINIMUM_MFA_PEER_DISCONNECT_THRESHOLD; + let response = client + .put("/api/v1/network/1") + .json(&location_data) + .send() + .await; + assert_eq!(response.status(), StatusCode::OK); +} + #[sqlx::test] async fn test_device(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; From dc810a1806ae3d03903c8dbcd39accb79349075b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Mon, 16 Mar 2026 17:44:45 +0100 Subject: [PATCH 4/5] update deps --- flake.lock | 12 ++++++------ web/package.json | 2 +- web/pnpm-lock.yaml | 26 +++++++++++++------------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/flake.lock b/flake.lock index 4542de30ed..d909857580 100644 --- a/flake.lock +++ b/flake.lock @@ -32,11 +32,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1772963539, - "narHash": "sha256-9jVDGZnvCckTGdYT53d/EfznygLskyLQXYwJLKMPsZs=", + "lastModified": 1773646010, + "narHash": "sha256-iYrs97hS7p5u4lQzuNWzuALGIOdkPXvjz7bviiBjUu8=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "9dcb002ca1690658be4a04645215baea8b95f31d", + "rev": "5b2c2d84341b2afb5647081c1386a80d7a8d8605", "type": "github" }, "original": { @@ -74,11 +74,11 @@ ] }, "locked": { - "lastModified": 1773115373, - "narHash": "sha256-bfK9FJFcQth6f3ydYggS5m0z2NRGF/PY6Y2XgZDJ6pg=", + "lastModified": 1773630837, + "narHash": "sha256-zJhgAGnbVKeBMJOb9ctZm4BGS/Rnrz+5lfSXTVah4HQ=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "1924b4672a2b8e4aee6e6652ec2e59a8d3c5648e", + "rev": "f600ea449c7b5bb596fa1cf21c871cc5b9e31316", "type": "github" }, "original": { diff --git a/web/package.json b/web/package.json index 6e6c384bd4..25be4800eb 100644 --- a/web/package.json +++ b/web/package.json @@ -34,7 +34,7 @@ "humanize-duration": "^3.33.2", "ipaddr.js": "^2.3.0", "lodash-es": "^4.17.23", - "motion": "^12.36.0", + "motion": "^12.37.0", "qrcode.react": "^4.2.0", "qs": "^6.15.0", "radashi": "^12.7.2", diff --git a/web/pnpm-lock.yaml b/web/pnpm-lock.yaml index fbec0df2f3..c2348267d7 100644 --- a/web/pnpm-lock.yaml +++ b/web/pnpm-lock.yaml @@ -72,8 +72,8 @@ importers: specifier: ^4.17.23 version: 4.17.23 motion: - specifier: ^12.36.0 - version: 12.36.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) + specifier: ^12.37.0 + version: 12.37.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) qrcode.react: specifier: ^4.2.0 version: 4.2.0(react@19.2.4) @@ -1775,8 +1775,8 @@ packages: fraction.js@5.3.4: resolution: {integrity: sha512-1X1NTtiJphryn/uLQz3whtY6jK3fTqoE3ohKs0tT+Ujr1W59oopxmoEh7Lu5p6vBaPbgoM0bzveAW4Qi5RyWDQ==} - framer-motion@12.36.0: - resolution: {integrity: sha512-4PqYHAT7gev0ke0wos+PyrcFxI0HScjm3asgU8nSYa8YzJFuwgIvdj3/s3ZaxLq0bUSboIn19A2WS/MHwLCvfw==} + framer-motion@12.37.0: + resolution: {integrity: sha512-j/QUcZS9Nw3NzZWoAbkzr3ETRFHyVeQMlGOUYUmG15U+uiyn9DqIktYruVPDcqY8I35qYR70JaZBvFmS6p+Pdg==} peerDependencies: '@emotion/is-prop-valid': '*' react: ^18.0.0 || ^19.0.0 @@ -2250,14 +2250,14 @@ packages: resolution: {integrity: sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw==} engines: {node: '>= 0.6'} - motion-dom@12.36.0: - resolution: {integrity: sha512-Ep1pq8P88rGJ75om8lTCA13zqd7ywPGwCqwuWwin6BKc0hMLkVfcS6qKlRqEo2+t0DwoUcgGJfXwaiFn4AOcQA==} + motion-dom@12.37.0: + resolution: {integrity: sha512-LnppZuwX1jQizRWTl9LBLMN3RbAEmdQkX/2Af0UW70NCqYJI/7GfI83vQP9Ucel/Avc0Tf2ZWy8FHawuc0O6Vg==} motion-utils@12.36.0: resolution: {integrity: sha512-eHWisygbiwVvf6PZ1vhaHCLamvkSbPIeAYxWUuL3a2PD/TROgE7FvfHWTIH4vMl798QLfMw15nRqIaRDXTlYRg==} - motion@12.36.0: - resolution: {integrity: sha512-5BMQuktYUX8aEByKWYx5tR4X3G08H2OMgp46wTxZ4o7CDDstyy4A0fe9RLNMjZiwvntCWGDvs16sC87/emz4Yw==} + motion@12.37.0: + resolution: {integrity: sha512-Ph6oyO5hGSIAPjDsqwchEP+EKXjyFK0ci6FTIFBbx+qaMl8zLzLzPLzd9q3DKhAHcvnV7LxQonMyA+FyAv9+gA==} peerDependencies: '@emotion/is-prop-valid': '*' react: ^18.0.0 || ^19.0.0 @@ -4338,9 +4338,9 @@ snapshots: fraction.js@5.3.4: {} - framer-motion@12.36.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): + framer-motion@12.37.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): dependencies: - motion-dom: 12.36.0 + motion-dom: 12.37.0 motion-utils: 12.36.0 tslib: 2.8.1 optionalDependencies: @@ -4909,15 +4909,15 @@ snapshots: dependencies: mime-db: 1.52.0 - motion-dom@12.36.0: + motion-dom@12.37.0: dependencies: motion-utils: 12.36.0 motion-utils@12.36.0: {} - motion@12.36.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): + motion@12.37.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4): dependencies: - framer-motion: 12.36.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) + framer-motion: 12.37.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) tslib: 2.8.1 optionalDependencies: react: 19.2.4 From f8314a6bd7178ce3683cda05d1b551c98a217d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20W=C3=B3jcik?= Date: Tue, 17 Mar 2026 09:19:45 +0100 Subject: [PATCH 5/5] review fix --- crates/defguard_core/src/handlers/wireguard.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/defguard_core/src/handlers/wireguard.rs b/crates/defguard_core/src/handlers/wireguard.rs index 9a6bf53d56..d3e02bdf27 100644 --- a/crates/defguard_core/src/handlers/wireguard.rs +++ b/crates/defguard_core/src/handlers/wireguard.rs @@ -114,9 +114,9 @@ impl WireguardNetworkData { return Ok(()); } - Err(WebError::BadRequest( - "peer_disconnect_threshold must be at least 120 when location MFA is enabled".into(), - )) + Err(WebError::BadRequest(format!( + "peer_disconnect_threshold must be at least {MIN_PEER_DISCONNECT_THRESHOLD_WITH_MFA} when location MFA is enabled" + ))) } pub(crate) async fn validate_location_mfa_mode<'e, E: sqlx::PgExecutor<'e>>(