From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 642FBF0B9 for ; Thu, 20 Jul 2023 16:33:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1238714D80 for ; Thu, 20 Jul 2023 16:33:29 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Thu, 20 Jul 2023 16:33:26 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id C271741FB8 for ; Thu, 20 Jul 2023 16:33:09 +0200 (CEST) From: Lukas Wagner To: pve-devel@lists.proxmox.com Date: Thu, 20 Jul 2023 16:31:50 +0200 Message-Id: <20230720143236.652292-24-l.wagner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230720143236.652292-1-l.wagner@proxmox.com> References: <20230720143236.652292-1-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.072 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: [pve-devel] [PATCH v4 proxmox 23/69] notify: ensure that filter/group/endpoint names are unique X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jul 2023 14:33:42 -0000 Otherwise, a filter with the same name as an already existing endpoint or group can overwrite it. Signed-off-by: Lukas Wagner --- proxmox-notify/src/api/filter.rs | 7 +---- proxmox-notify/src/api/gotify.rs | 10 +------ proxmox-notify/src/api/group.rs | 24 ++------------- proxmox-notify/src/api/mod.rs | 47 ++++++++++++++++++++++++++++-- proxmox-notify/src/api/sendmail.rs | 7 +---- 5 files changed, 51 insertions(+), 44 deletions(-) diff --git a/proxmox-notify/src/api/filter.rs b/proxmox-notify/src/api/filter.rs index 824f802d..b5b62849 100644 --- a/proxmox-notify/src/api/filter.rs +++ b/proxmox-notify/src/api/filter.rs @@ -31,12 +31,7 @@ pub fn get_filter(config: &Config, name: &str) -> Result /// Returns an `ApiError` if a filter with the same name already exists or /// if the filter could not be saved. pub fn add_filter(config: &mut Config, filter_config: &FilterConfig) -> Result<(), ApiError> { - if get_filter(config, &filter_config.name).is_ok() { - return Err(ApiError::bad_request( - format!("filter '{}' already exists", filter_config.name), - None, - )); - } + super::ensure_unique(config, &filter_config.name)?; config .config diff --git a/proxmox-notify/src/api/gotify.rs b/proxmox-notify/src/api/gotify.rs index 5c4db4be..521dd167 100644 --- a/proxmox-notify/src/api/gotify.rs +++ b/proxmox-notify/src/api/gotify.rs @@ -43,15 +43,7 @@ pub fn add_endpoint( panic!("name for endpoint config and private config must be identical"); } - if super::endpoint_exists(config, &endpoint_config.name) { - return Err(ApiError::bad_request( - format!( - "endpoint with name '{}' already exists!", - endpoint_config.name - ), - None, - )); - } + super::ensure_unique(config, &endpoint_config.name)?; if let Some(filter) = &endpoint_config.filter { // Check if filter exists diff --git a/proxmox-notify/src/api/group.rs b/proxmox-notify/src/api/group.rs index fe3de12f..d6b8f38d 100644 --- a/proxmox-notify/src/api/group.rs +++ b/proxmox-notify/src/api/group.rs @@ -31,12 +31,7 @@ pub fn get_group(config: &Config, name: &str) -> Result { /// Returns an `ApiError` if a group with the same name already exists, or /// if the group could not be saved pub fn add_group(config: &mut Config, group_config: &GroupConfig) -> Result<(), ApiError> { - if get_group(config, &group_config.name).is_ok() { - return Err(ApiError::bad_request( - format!("group '{}' already exists", group_config.name), - None, - )); - } + super::ensure_unique(config, &group_config.name)?; if group_config.endpoint.is_empty() { return Err(ApiError::bad_request( @@ -50,7 +45,7 @@ pub fn add_group(config: &mut Config, group_config: &GroupConfig) -> Result<(), super::filter::get_filter(config, filter)?; } - check_if_endpoints_exist(config, &group_config.endpoint)?; + super::ensure_endpoints_exist(config, &group_config.endpoint)?; config .config @@ -91,7 +86,7 @@ pub fn update_group( } if let Some(endpoints) = &updater.endpoint { - check_if_endpoints_exist(config, endpoints)?; + super::ensure_endpoints_exist(config, endpoints)?; if endpoints.is_empty() { return Err(ApiError::bad_request( "group must contain at least one endpoint", @@ -138,19 +133,6 @@ pub fn delete_group(config: &mut Config, name: &str) -> Result<(), ApiError> { Ok(()) } -fn check_if_endpoints_exist(config: &Config, endpoints: &[String]) -> Result<(), ApiError> { - for endpoint in endpoints { - if !super::endpoint_exists(config, endpoint) { - return Err(ApiError::not_found( - format!("endoint '{endpoint}' does not exist"), - None, - )); - } - } - - Ok(()) -} - // groups cannot be empty, so only build the tests if we have the // sendmail endpoint available #[cfg(all(test, feature = "sendmail"))] diff --git a/proxmox-notify/src/api/mod.rs b/proxmox-notify/src/api/mod.rs index e064b607..9a59719b 100644 --- a/proxmox-notify/src/api/mod.rs +++ b/proxmox-notify/src/api/mod.rs @@ -87,7 +87,7 @@ fn verify_digest(config: &Config, digest: Option<&[u8]>) -> Result<(), ApiError> Ok(()) } -fn endpoint_exists(config: &Config, name: &str) -> bool { +fn ensure_endpoint_exists(config: &Config, name: &str) -> Result<(), ApiError> { let mut exists = false; #[cfg(feature = "sendmail")] @@ -99,7 +99,33 @@ fn endpoint_exists(config: &Config, name: &str) -> bool { exists = exists || gotify::get_endpoint(config, name).is_ok(); } - exists + if !exists { + Err(ApiError::not_found( + format!("endpoint '{name}' does not exist"), + None, + )) + } else { + Ok(()) + } +} + +fn ensure_endpoints_exist>(config: &Config, endpoints: &[T]) -> Result<(), ApiError> { + for endpoint in endpoints { + ensure_endpoint_exists(config, endpoint.as_ref())?; + } + + Ok(()) +} + +fn ensure_unique(config: &Config, entity: &str) -> Result<(), ApiError> { + if config.config.sections.contains_key(entity) { + return Err(ApiError::bad_request( + format!("Cannot create '{entity}', an entity with the same name already exists"), + None, + )); + } + + Ok(()) } fn get_referrers(config: &Config, entity: &str) -> Result, ApiError> { @@ -324,4 +350,21 @@ mod tests { assert!(ensure_unused(&config, "sendmail").is_err()); assert!(ensure_unused(&config, "group").is_ok()); } + + #[test] + fn test_ensure_unique() { + let config = prepare_config().unwrap(); + + assert!(ensure_unique(&config, "sendmail").is_err()); + assert!(ensure_unique(&config, "group").is_err()); + assert!(ensure_unique(&config, "new").is_ok()); + } + + #[test] + fn test_ensure_endpoints_exist() { + let config = prepare_config().unwrap(); + + assert!(ensure_endpoints_exist(&config, &vec!["sendmail", "gotify"]).is_ok()); + assert!(ensure_endpoints_exist(&config, &vec!["group", "filter"]).is_err()); + } } diff --git a/proxmox-notify/src/api/sendmail.rs b/proxmox-notify/src/api/sendmail.rs index bf225f29..59a57b47 100644 --- a/proxmox-notify/src/api/sendmail.rs +++ b/proxmox-notify/src/api/sendmail.rs @@ -33,12 +33,7 @@ pub fn get_endpoint(config: &Config, name: &str) -> Result Result<(), ApiError> { - if super::endpoint_exists(config, &endpoint.name) { - return Err(ApiError::bad_request( - format!("endpoint with name '{}' already exists!", &endpoint.name), - None, - )); - } + super::ensure_unique(config, &endpoint.name)?; if let Some(filter) = &endpoint.filter { // Check if filter exists -- 2.39.2