Remove last Option<Vec.. from API. Fixes #2820 (#2822)

* Remove last Option<Vec.. from API. Fixes #2820

* Add empty allowed_instances check.

* Adding comment for allowed_instances.
This commit is contained in:
Dessalines 2023-04-21 17:41:03 -04:00 committed by GitHub
parent 1e26709cb4
commit eb40aeb89b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 38 additions and 45 deletions

View file

@ -250,8 +250,8 @@ pub struct LeaveAdmin {
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]
pub struct FederatedInstances { pub struct FederatedInstances {
pub linked: Vec<Instance>, pub linked: Vec<Instance>,
pub allowed: Option<Vec<Instance>>, pub allowed: Vec<Instance>,
pub blocked: Option<Vec<Instance>>, pub blocked: Vec<Instance>,
} }
#[derive(Debug, Serialize, Deserialize, Clone)] #[derive(Debug, Serialize, Deserialize, Clone)]

View file

@ -318,10 +318,6 @@ pub async fn build_federated_instances(
let allowed = Instance::allowlist(pool).await?; let allowed = Instance::allowlist(pool).await?;
let blocked = Instance::blocklist(pool).await?; let blocked = Instance::blocklist(pool).await?;
// These can return empty vectors, so convert them to options
let allowed = (!allowed.is_empty()).then_some(allowed);
let blocked = (!blocked.is_empty()).then_some(blocked);
Ok(Some(FederatedInstances { Ok(Some(FederatedInstances {
linked, linked,
allowed, allowed,

View file

@ -69,17 +69,23 @@ fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> Result
return Err("Federation disabled"); return Err("Federation disabled");
} }
if let Some(blocked) = local_site_data.blocked_instances.as_ref() { if local_site_data
if blocked.iter().any(|i| domain.eq(&i.domain)) { .blocked_instances
.iter()
.any(|i| domain.eq(&i.domain))
{
return Err("Domain is blocked"); return Err("Domain is blocked");
} }
}
if let Some(allowed) = local_site_data.allowed_instances.as_ref() { // Only check this if there are instances in the allowlist
if !allowed.iter().any(|i| domain.eq(&i.domain)) { if !local_site_data.allowed_instances.is_empty()
&& !local_site_data
.allowed_instances
.iter()
.any(|i| domain.eq(&i.domain))
{
return Err("Domain is not in allowlist"); return Err("Domain is not in allowlist");
} }
}
Ok(()) Ok(())
} }
@ -87,8 +93,8 @@ fn check_apub_id_valid(apub_id: &Url, local_site_data: &LocalSiteData) -> Result
#[derive(Clone)] #[derive(Clone)]
pub(crate) struct LocalSiteData { pub(crate) struct LocalSiteData {
local_site: Option<LocalSite>, local_site: Option<LocalSite>,
allowed_instances: Option<Vec<Instance>>, allowed_instances: Vec<Instance>,
blocked_instances: Option<Vec<Instance>>, blocked_instances: Vec<Instance>,
} }
pub(crate) async fn fetch_local_site_data( pub(crate) async fn fetch_local_site_data(
@ -96,12 +102,8 @@ pub(crate) async fn fetch_local_site_data(
) -> Result<LocalSiteData, diesel::result::Error> { ) -> Result<LocalSiteData, diesel::result::Error> {
// LocalSite may be missing // LocalSite may be missing
let local_site = LocalSite::read(pool).await.ok(); let local_site = LocalSite::read(pool).await.ok();
let allowed = Instance::allowlist(pool).await?; let allowed_instances = Instance::allowlist(pool).await?;
let blocked = Instance::blocklist(pool).await?; let blocked_instances = Instance::blocklist(pool).await?;
// These can return empty vectors, so convert them to options
let allowed_instances = (!allowed.is_empty()).then_some(allowed);
let blocked_instances = (!blocked.is_empty()).then_some(blocked);
Ok(LocalSiteData { Ok(LocalSiteData {
local_site, local_site,
@ -126,12 +128,12 @@ pub(crate) fn check_apub_id_valid_with_strictness(
} }
check_apub_id_valid(apub_id, local_site_data).map_err(LemmyError::from_message)?; check_apub_id_valid(apub_id, local_site_data).map_err(LemmyError::from_message)?;
if let Some(allowed) = local_site_data.allowed_instances.as_ref() { // Only check allowlist if this is a community, and there are instances in the allowlist
// Only check allowlist if this is a community if is_strict && !local_site_data.allowed_instances.is_empty() {
if is_strict {
// need to allow this explicitly because apub receive might contain objects from our local // need to allow this explicitly because apub receive might contain objects from our local
// instance. // instance.
let mut allowed_and_local = allowed let mut allowed_and_local = local_site_data
.allowed_instances
.iter() .iter()
.map(|i| i.domain.clone()) .map(|i| i.domain.clone())
.collect::<Vec<String>>(); .collect::<Vec<String>>();
@ -147,7 +149,6 @@ pub(crate) fn check_apub_id_valid_with_strictness(
)); ));
} }
} }
}
Ok(()) Ok(())
} }

View file

@ -64,7 +64,6 @@ pub(crate) mod tests {
}; };
use lemmy_db_schema::{source::secret::Secret, utils::build_db_pool_for_tests}; use lemmy_db_schema::{source::secret::Secret, utils::build_db_pool_for_tests};
use lemmy_utils::{ use lemmy_utils::{
error::LemmyError,
rate_limit::{RateLimitCell, RateLimitConfig}, rate_limit::{RateLimitCell, RateLimitConfig},
settings::SETTINGS, settings::SETTINGS,
}; };
@ -89,9 +88,6 @@ pub(crate) mod tests {
// TODO: would be nice if we didnt have to use a full context for tests. // TODO: would be nice if we didnt have to use a full context for tests.
pub(crate) async fn init_context() -> Data<LemmyContext> { pub(crate) async fn init_context() -> Data<LemmyContext> {
async fn x() -> Result<String, LemmyError> {
Ok(String::new())
}
// call this to run migrations // call this to run migrations
let pool = build_db_pool_for_tests().await; let pool = build_db_pool_for_tests().await;