diff --git a/crates/defguard_core/src/enterprise/db/models/acl.rs b/crates/defguard_core/src/enterprise/db/models/acl.rs index c876dff947..efdbca7d5a 100644 --- a/crates/defguard_core/src/enterprise/db/models/acl.rs +++ b/crates/defguard_core/src/enterprise/db/models/acl.rs @@ -854,7 +854,7 @@ impl TryFrom for AclRule { any_address: rule.any_address, any_port: rule.any_port, any_protocol: rule.any_protocol, - use_manual_destination_settings: true, + use_manual_destination_settings: rule.use_manual_destination_settings, }) } } @@ -1644,9 +1644,9 @@ impl TryFrom<&EditAclAlias> for AclAlias { kind: AliasKind::Component, state: AliasState::Applied, protocols: alias.protocols.clone(), - any_address: true, - any_port: true, - any_protocol: true, + any_address: false, + any_port: false, + any_protocol: false, }) } } diff --git a/crates/defguard_core/src/enterprise/handlers/acl.rs b/crates/defguard_core/src/enterprise/handlers/acl.rs index ff7e8c2cfd..5bb9973d2d 100644 --- a/crates/defguard_core/src/enterprise/handlers/acl.rs +++ b/crates/defguard_core/src/enterprise/handlers/acl.rs @@ -131,7 +131,24 @@ pub struct EditAclRule { impl EditAclRule { pub fn validate(&self) -> Result<(), WebError> { - // FIXME: validate that destination is defined + let manual_configured = self.any_address + || self.any_port + || self.any_protocol + || !self.addresses.trim().is_empty() + || !self.ports.trim().is_empty() + || !self.protocols.is_empty(); + if self.use_manual_destination_settings { + if !manual_configured { + return Err(WebError::BadRequest( + "Must provide manual destination settings".to_string(), + )); + } + } else if self.destinations.is_empty() { + return Err(WebError::BadRequest( + "Must provide destination alias".to_string(), + )); + } + // check if some allowed users/group/devices are configured if !self.allow_all_users && !self.allow_all_groups diff --git a/crates/defguard_core/src/enterprise/handlers/acl/destination.rs b/crates/defguard_core/src/enterprise/handlers/acl/destination.rs index 1df5d43c71..f26172e973 100644 --- a/crates/defguard_core/src/enterprise/handlers/acl/destination.rs +++ b/crates/defguard_core/src/enterprise/handlers/acl/destination.rs @@ -16,6 +16,7 @@ use crate::{ AclAlias, AclAliasDestinationRange, AclAliasInfo, AclError, AliasKind, AliasState, Protocol, acl_delete_related_objects, parse_destination_addresses, }, + error::WebError, handlers::{ApiResponse, ApiResult}, }; @@ -32,6 +33,26 @@ pub(crate) struct EditAclDestination { } impl EditAclDestination { + fn validate(&self) -> Result<(), WebError> { + if !self.any_address && self.addresses.trim().is_empty() { + return Err(WebError::BadRequest( + "Must provide destination addresses or enable any address".to_string(), + )); + } + if !self.any_port && self.ports.trim().is_empty() { + return Err(WebError::BadRequest( + "Must provide destination ports or enable any port".to_string(), + )); + } + if !self.any_protocol && self.protocols.is_empty() { + return Err(WebError::BadRequest( + "Must provide destination protocols or enable any protocol".to_string(), + )); + } + + Ok(()) + } + /// Creates relation objects for a given [`AclAlias`] based on [`AclAliasInfo`] object. pub(crate) async fn create_related_objects( &self, @@ -307,6 +328,7 @@ pub(crate) async fn create_acl_destination( "User {} creating ACL destination {data:?}", session.user.username ); + data.validate()?; let alias = ApiAclDestination::create_from_api(&appstate.pool, &data) .await .map_err(|err| { @@ -344,6 +366,7 @@ pub(crate) async fn update_acl_destination( "User {} updating ACL destination {data:?}", session.user.username ); + data.validate()?; let alias = ApiAclDestination::update_from_api(&appstate.pool, id, &data) .await .map_err(|err| { diff --git a/crates/defguard_core/tests/integration/api/acl.rs b/crates/defguard_core/tests/integration/api/acl.rs index 36f913ebda..eef7df3200 100644 --- a/crates/defguard_core/tests/integration/api/acl.rs +++ b/crates/defguard_core/tests/integration/api/acl.rs @@ -218,6 +218,111 @@ async fn test_rule_crud(_: PgPoolOptions, options: PgConnectOptions) { assert_eq!(response_rules.len(), 0); } +#[sqlx::test] +async fn test_rule_requires_destination(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + + let (mut client, _) = make_test_client(pool).await; + authenticate_admin(&mut client).await; + + // manual destination enabled but empty + let mut rule = make_rule(); + rule.use_manual_destination_settings = true; + rule.addresses = String::new(); + rule.ports = String::new(); + rule.protocols = Vec::new(); + rule.any_address = false; + rule.any_port = false; + rule.any_protocol = false; + rule.destinations = Vec::new(); + let response = client.post("/api/v1/acl/rule").json(&rule).send().await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // manual destination disabled and no destination aliases + let mut rule = make_rule(); + rule.use_manual_destination_settings = false; + rule.addresses = String::new(); + rule.ports = String::new(); + rule.protocols = Vec::new(); + rule.any_address = false; + rule.any_port = false; + rule.any_protocol = false; + rule.destinations = Vec::new(); + let response = client.post("/api/v1/acl/rule").json(&rule).send().await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // manual destination configured + let mut rule = make_rule(); + rule.use_manual_destination_settings = true; + rule.addresses = "10.0.0.1".to_string(); + rule.ports = "80".to_string(); + rule.protocols = vec![6]; + rule.any_address = false; + rule.any_port = false; + rule.any_protocol = false; + rule.destinations = Vec::new(); + let response = client.post("/api/v1/acl/rule").json(&rule).send().await; + assert_eq!(response.status(), StatusCode::CREATED); + let created_rule: ApiAclRule = response.json().await; + + // destination alias configured + let destination = make_destination(); + let response = client + .post("/api/v1/acl/destination") + .json(&destination) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + let destination: Value = response.json().await; + let destination_id = destination["id"].as_i64().unwrap(); + + let mut rule = make_rule(); + rule.use_manual_destination_settings = false; + rule.addresses = String::new(); + rule.ports = String::new(); + rule.protocols = Vec::new(); + rule.any_address = false; + rule.any_port = false; + rule.any_protocol = false; + rule.destinations = vec![destination_id]; + let response = client.post("/api/v1/acl/rule").json(&rule).send().await; + assert_eq!(response.status(), StatusCode::CREATED); + + // update to invalid manual destination + let mut invalid_update = created_rule.clone(); + invalid_update.use_manual_destination_settings = true; + invalid_update.addresses = String::new(); + invalid_update.ports = String::new(); + invalid_update.protocols = Vec::new(); + invalid_update.any_address = false; + invalid_update.any_port = false; + invalid_update.any_protocol = false; + invalid_update.destinations = Vec::new(); + let response = client + .put(format!("/api/v1/acl/rule/{}", created_rule.id)) + .json(&invalid_update) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // update to invalid alias-only destination + let mut invalid_update = created_rule.clone(); + invalid_update.use_manual_destination_settings = false; + invalid_update.addresses = String::new(); + invalid_update.ports = String::new(); + invalid_update.protocols = Vec::new(); + invalid_update.any_address = false; + invalid_update.any_port = false; + invalid_update.any_protocol = false; + invalid_update.destinations = Vec::new(); + let response = client + .put(format!("/api/v1/acl/rule/{}", created_rule.id)) + .json(&invalid_update) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); +} + #[sqlx::test] async fn test_rule_enterprise(_: PgPoolOptions, options: PgConnectOptions) { let pool = setup_pool(options).await; @@ -1364,3 +1469,107 @@ async fn test_acl_count_endpoints(_: PgPoolOptions, options: PgConnectOptions) { assert_eq!(counts["applied"], json!(2)); assert_eq!(counts["pending"], json!(1)); } + +#[sqlx::test] +async fn test_destination_requires_any_or_values(_: PgPoolOptions, options: PgConnectOptions) { + let pool = setup_pool(options).await; + + let (mut client, _) = make_test_client(pool).await; + authenticate_admin(&mut client).await; + + // create destination with empty fields and no any flags + let invalid_destination = json!({ + "name": "invalid destination", + "addresses": "", + "ports": "", + "protocols": [], + "any_address": false, + "any_port": false, + "any_protocol": false + }); + let response = client + .post("/api/v1/acl/destination") + .json(&invalid_destination) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // try to create destinations with only some destination fields set + let invalid_destination = json!({ + "name": "invalid destination", + "addresses": "", + "ports": "22, 443", + "protocols": [], + "any_address": false, + "any_port": false, + "any_protocol": true + }); + let response = client + .post("/api/v1/acl/destination") + .json(&invalid_destination) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // create valid destination + let destination = make_destination(); + let response = client + .post("/api/v1/acl/destination") + .json(&destination) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); + let destination: Value = response.json().await; + let destination_id = destination["id"].as_i64().unwrap(); + + // update destination with empty fields and no any flags + let invalid_update = json!({ + "name": "invalid update", + "addresses": "", + "ports": "", + "protocols": [], + "any_address": false, + "any_port": false, + "any_protocol": false + }); + let response = client + .put(format!("/api/v1/acl/destination/{destination_id}")) + .json(&invalid_update) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // update destination with some destination fields set + let invalid_update = json!({ + "name": "invalid update", + "addresses": "", + "ports": "5432", + "protocols": [], + "any_address": true, + "any_port": false, + "any_protocol": false + }); + let response = client + .put(format!("/api/v1/acl/destination/{destination_id}")) + .json(&invalid_update) + .send() + .await; + assert_eq!(response.status(), StatusCode::BAD_REQUEST); + + // create valid destination with only "any" flags enabled + let destination = json!({ + "name": "valid destination", + "addresses": "", + "ports": "", + "protocols": [], + "any_address": true, + "any_port": true, + "any_protocol": true + }); + let response = client + .post("/api/v1/acl/destination") + .json(&destination) + .send() + .await; + assert_eq!(response.status(), StatusCode::CREATED); +} diff --git a/web/src/pages/CERulePage/CERulePage.tsx b/web/src/pages/CERulePage/CERulePage.tsx index 57dd5b4522..526e187ccc 100644 --- a/web/src/pages/CERulePage/CERulePage.tsx +++ b/web/src/pages/CERulePage/CERulePage.tsx @@ -34,10 +34,12 @@ import { Checkbox } from '../../shared/defguard-ui/components/Checkbox/Checkbox' import { CheckboxIndicator } from '../../shared/defguard-ui/components/CheckboxIndicator/CheckboxIndicator'; import { Chip } from '../../shared/defguard-ui/components/Chip/Chip'; import { Divider } from '../../shared/defguard-ui/components/Divider/Divider'; +import { FieldError } from '../../shared/defguard-ui/components/FieldError/FieldError'; import { Fold } from '../../shared/defguard-ui/components/Fold/Fold'; import { Icon, type IconKindValue } from '../../shared/defguard-ui/components/Icon'; import { MarkedSection } from '../../shared/defguard-ui/components/MarkedSection/MarkedSection'; import { SizedBox } from '../../shared/defguard-ui/components/SizedBox/SizedBox'; +import { useFormFieldError } from '../../shared/defguard-ui/hooks/useFormFieldError'; import { Snackbar } from '../../shared/defguard-ui/providers/snackbar/snackbar'; import { TooltipContent } from '../../shared/defguard-ui/providers/tooltip/TooltipContent'; import { TooltipProvider } from '../../shared/defguard-ui/providers/tooltip/TooltipContext'; @@ -376,6 +378,38 @@ const Content = ({ rule: initialRule }: Props) => { message, }); } + + if (vals.use_manual_destination_settings) { + const message = + 'Manual destination is enabled. Provide a value or enable Any.'; + if (!vals.any_address && vals.addresses.trim().length === 0) { + ctx.addIssue({ + path: ['addresses'], + code: 'custom', + message, + }); + } + if (!vals.any_port && vals.ports.trim().length === 0) { + ctx.addIssue({ + path: ['ports'], + code: 'custom', + message, + }); + } + if (!vals.any_protocol && vals.protocols.size === 0) { + ctx.addIssue({ + path: ['protocols'], + code: 'custom', + message, + }); + } + } else if (vals.destinations.size === 0) { + ctx.addIssue({ + path: ['destinations'], + code: 'custom', + message: m.form_error_required(), + }); + } }), [], ); @@ -551,6 +585,7 @@ const Content = ({ rule: initialRule }: Props) => { }); }} /> + {selectedDestinations.length > 0 && (
@@ -1036,3 +1071,11 @@ const AliasDataBlock = ({ values }: AliasDataBlockProps) => {
); }; + +const DestinationSelectionError = () => { + const error = useFormFieldError(); + if (!error) return null; + return ( + + ); +};