From da3e3b6d8a6684d58eecef91c675a696005f8330 Mon Sep 17 00:00:00 2001 From: Scott <97430840+scme0@users.noreply.github.com> Date: Thu, 22 Jun 2023 05:44:12 +0930 Subject: [PATCH] Ensure site can only either be a private instance or federated when creating or editing site (#3237) * Add site visibility validation * Fix formatting * linter changes * Update error message to match existing check * Remove existing check --------- Co-authored-by: Scott Merchant <97430840+scottmerchant@users.noreply.github.com> --- crates/api_crud/src/site/create.rs | 15 ++++++++++---- crates/api_crud/src/site/update.rs | 25 ++++++++++------------ crates/utils/src/utils/validation.rs | 31 ++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 18 deletions(-) diff --git a/crates/api_crud/src/site/create.rs b/crates/api_crud/src/site/create.rs index af854066..2a51309a 100644 --- a/crates/api_crud/src/site/create.rs +++ b/crates/api_crud/src/site/create.rs @@ -29,7 +29,7 @@ use lemmy_utils::{ error::LemmyError, utils::{ slurs::{check_slurs, check_slurs_opt}, - validation::is_valid_body_field, + validation::{check_site_visibility_valid, is_valid_body_field}, }, }; use url::Url; @@ -50,6 +50,16 @@ impl PerformCrud for CreateSite { let local_user_view = local_user_view_from_jwt(&data.auth, context).await?; + // Make sure user is an admin + is_admin(&local_user_view)?; + + check_site_visibility_valid( + local_site.private_instance, + local_site.federation_enabled, + &data.private_instance, + &data.federation_enabled, + )?; + let sidebar = diesel_option_overwrite(&data.sidebar); let description = diesel_option_overwrite(&data.description); let icon = diesel_option_overwrite_to_url(&data.icon)?; @@ -59,9 +69,6 @@ impl PerformCrud for CreateSite { check_slurs(&data.name, &slur_regex)?; check_slurs_opt(&data.description, &slur_regex)?; - // Make sure user is an admin - is_admin(&local_user_view)?; - if let Some(Some(desc)) = &description { site_description_length_check(desc)?; } diff --git a/crates/api_crud/src/site/update.rs b/crates/api_crud/src/site/update.rs index ac1544ff..fadde0a0 100644 --- a/crates/api_crud/src/site/update.rs +++ b/crates/api_crud/src/site/update.rs @@ -30,7 +30,10 @@ use lemmy_db_schema::{ use lemmy_db_views::structs::SiteView; use lemmy_utils::{ error::LemmyError, - utils::{slurs::check_slurs_opt, validation::is_valid_body_field}, + utils::{ + slurs::check_slurs_opt, + validation::{check_site_visibility_valid, is_valid_body_field}, + }, }; #[async_trait::async_trait(?Send)] @@ -48,6 +51,13 @@ impl PerformCrud for EditSite { // Make sure user is an admin is_admin(&local_user_view)?; + check_site_visibility_valid( + local_site.private_instance, + local_site.federation_enabled, + &data.private_instance, + &data.federation_enabled, + )?; + let slur_regex = local_site_to_slur_regex(&local_site); check_slurs_opt(&data.name, &slur_regex)?; @@ -76,19 +86,6 @@ impl PerformCrud for EditSite { } } - let enabled_private_instance_with_federation = data.private_instance == Some(true) - && data - .federation_enabled - .unwrap_or(local_site.federation_enabled); - let enabled_federation_with_private_instance = data.federation_enabled == Some(true) - && data.private_instance.unwrap_or(local_site.private_instance); - - if enabled_private_instance_with_federation || enabled_federation_with_private_instance { - return Err(LemmyError::from_message( - "cant_enable_private_instance_and_federation_together", - )); - } - if let Some(discussion_languages) = data.discussion_languages.clone() { SiteLanguage::update(context.pool(), discussion_languages.clone(), &site).await?; } diff --git a/crates/utils/src/utils/validation.rs b/crates/utils/src/utils/validation.rs index c4feb467..41103332 100644 --- a/crates/utils/src/utils/validation.rs +++ b/crates/utils/src/utils/validation.rs @@ -149,10 +149,29 @@ pub fn build_totp_2fa(site_name: &str, username: &str, secret: &str) -> Result, + new_federation_enabled: &Option, +) -> LemmyResult<()> { + let private_instance = new_private_instance.unwrap_or(current_private_instance); + let federation_enabled = new_federation_enabled.unwrap_or(current_federation_enabled); + + if private_instance && federation_enabled { + return Err(LemmyError::from_message( + "cant_enable_private_instance_and_federation_together", + )); + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::build_totp_2fa; use crate::utils::validation::{ + check_site_visibility_valid, clean_url_params, generate_totp_2fa_secret, is_valid_actor_name, @@ -226,4 +245,16 @@ mod tests { let totp = build_totp_2fa("lemmy", "my_name", &generated_secret); assert!(totp.is_ok()); } + + #[test] + fn test_check_site_visibility_valid() { + assert!(check_site_visibility_valid(true, true, &None, &None).is_err()); + assert!(check_site_visibility_valid(true, false, &None, &Some(true)).is_err()); + assert!(check_site_visibility_valid(false, true, &Some(true), &None).is_err()); + assert!(check_site_visibility_valid(false, false, &Some(true), &Some(true)).is_err()); + assert!(check_site_visibility_valid(true, false, &None, &None).is_ok()); + assert!(check_site_visibility_valid(false, true, &None, &None).is_ok()); + assert!(check_site_visibility_valid(false, false, &Some(true), &None).is_ok()); + assert!(check_site_visibility_valid(false, false, &None, &Some(true)).is_ok()); + } } -- 2.44.1