From 110167f085e4eb0d36b3c4122b674af7a3a59109 Mon Sep 17 00:00:00 2001 From: dessalines Date: Fri, 15 Jan 2021 01:18:18 +0000 Subject: [PATCH] Ensure that comments are always delivered to the parent creator (fixes #1325) (#156) Don't send activities to ourselves Ensure that comments are always delivered to the parent creator (fixes #1325) Co-authored-by: Felix Ableitner Reviewed-on: https://yerbamate.ml/LemmyNet/lemmy/pulls/156 Co-Authored-By: dessalines Co-Committed-By: dessalines --- lemmy_apub/src/activities/send/comment.rs | 46 ++++++++++------------- lemmy_apub/src/activity_queue.rs | 9 ++++- lemmy_apub/src/lib.rs | 8 +--- lemmy_utils/src/settings.rs | 17 +++++++++ 4 files changed, 45 insertions(+), 35 deletions(-) diff --git a/lemmy_apub/src/activities/send/comment.rs b/lemmy_apub/src/activities/send/comment.rs index 36917ecb2..e3c622edb 100644 --- a/lemmy_apub/src/activities/send/comment.rs +++ b/lemmy_apub/src/activities/send/comment.rs @@ -57,17 +57,14 @@ impl ApubObjectType for Comment { }) .await??; - let mut maa = collect_non_local_mentions_and_addresses(&self.content, context).await?; - let mut ccs = vec![community.actor_id()?]; - ccs.append(&mut maa.addressed_ccs); - ccs.push(get_comment_parent_creator_id(context.pool(), &self).await?); + let maa = collect_non_local_mentions(&self, &community, context).await?; let mut create = Create::new(creator.actor_id.to_owned(), note.into_any_base()?); create .set_many_contexts(lemmy_context()?) .set_id(generate_activity_id(CreateType::Create)?) .set_to(public()) - .set_many_ccs(ccs) + .set_many_ccs(maa.ccs.to_owned()) // Set the mention tags .set_many_tags(maa.get_tags()?); @@ -90,17 +87,14 @@ impl ApubObjectType for Comment { }) .await??; - let mut maa = collect_non_local_mentions_and_addresses(&self.content, context).await?; - let mut ccs = vec![community.actor_id()?]; - ccs.append(&mut maa.addressed_ccs); - ccs.push(get_comment_parent_creator_id(context.pool(), &self).await?); + let maa = collect_non_local_mentions(&self, &community, context).await?; let mut update = Update::new(creator.actor_id.to_owned(), note.into_any_base()?); update .set_many_contexts(lemmy_context()?) .set_id(generate_activity_id(UpdateType::Update)?) .set_to(public()) - .set_many_ccs(ccs) + .set_many_ccs(maa.ccs.to_owned()) // Set the mention tags .set_many_tags(maa.get_tags()?); @@ -295,7 +289,7 @@ impl ApubLikeableType for Comment { } struct MentionsAndAddresses { - addressed_ccs: Vec, + ccs: Vec, inboxes: Vec, tags: Vec, } @@ -313,23 +307,26 @@ impl MentionsAndAddresses { /// This takes a comment, and builds a list of to_addresses, inboxes, /// and mention tags, so they know where to be sent to. /// Addresses are the users / addresses that go in the cc field. -async fn collect_non_local_mentions_and_addresses( - content: &str, +async fn collect_non_local_mentions( + comment: &Comment, + community: &Community, context: &LemmyContext, ) -> Result { - let mut addressed_ccs = vec![]; + let parent_creator = get_comment_parent_creator(context.pool(), comment).await?; + let mut addressed_ccs = vec![community.actor_id()?, parent_creator.actor_id()?]; + // Note: dont include community inbox here, as we send to it separately with `send_to_community()` + let mut inboxes = vec![parent_creator.get_shared_inbox_url()?]; // Add the mention tag let mut tags = Vec::new(); - // Get the inboxes for any mentions - let mentions = scrape_text_for_mentions(&content) + // Get the user IDs for any mentions + let mentions = scrape_text_for_mentions(&comment.content) .into_iter() // Filter only the non-local ones .filter(|m| !m.is_local()) .collect::>(); - let mut mention_inboxes: Vec = Vec::new(); for mention in &mentions { // TODO should it be fetching it every time? if let Ok(actor_id) = fetch_webfinger_url(mention, context.client()).await { @@ -337,19 +334,18 @@ async fn collect_non_local_mentions_and_addresses( addressed_ccs.push(actor_id.to_owned().to_string().parse()?); let mention_user = get_or_fetch_and_upsert_user(&actor_id, context, &mut 0).await?; - let shared_inbox = mention_user.get_shared_inbox_url()?; + inboxes.push(mention_user.get_shared_inbox_url()?); - mention_inboxes.push(shared_inbox); let mut mention_tag = Mention::new(); mention_tag.set_href(actor_id).set_name(mention.full_name()); tags.push(mention_tag); } } - let inboxes = mention_inboxes.into_iter().unique().collect(); + let inboxes = inboxes.into_iter().unique().collect(); Ok(MentionsAndAddresses { - addressed_ccs, + ccs: addressed_ccs, inboxes, tags, }) @@ -357,10 +353,7 @@ async fn collect_non_local_mentions_and_addresses( /// Returns the apub ID of the user this comment is responding to. Meaning, in case this is a /// top-level comment, the creator of the post, otherwise the creator of the parent comment. -async fn get_comment_parent_creator_id( - pool: &DbPool, - comment: &Comment, -) -> Result { +async fn get_comment_parent_creator(pool: &DbPool, comment: &Comment) -> Result { let parent_creator_id = if let Some(parent_comment_id) = comment.parent_id { let parent_comment = blocking(pool, move |conn| Comment::read(conn, parent_comment_id)).await??; @@ -370,8 +363,7 @@ async fn get_comment_parent_creator_id( let parent_post = blocking(pool, move |conn| Post::read(conn, parent_post_id)).await??; parent_post.creator_id }; - let parent_creator = blocking(pool, move |conn| User_::read(conn, parent_creator_id)).await??; - Ok(parent_creator.actor_id()?) + Ok(blocking(pool, move |conn| User_::read(conn, parent_creator_id)).await??) } /// Turns a user id like `@name@example.com` into an apub ID, like `https://example.com/user/name`, diff --git a/lemmy_apub/src/activity_queue.rs b/lemmy_apub/src/activity_queue.rs index a8f2ab180..ff792c4da 100644 --- a/lemmy_apub/src/activity_queue.rs +++ b/lemmy_apub/src/activity_queue.rs @@ -219,6 +219,13 @@ where return Ok(()); } + // Don't send anything to ourselves + let hostname = Settings::get().get_hostname_without_port()?; + let inboxes: Vec<&Url> = inboxes + .iter() + .filter(|i| i.domain().unwrap() != hostname) + .collect(); + let activity = activity.into_any_base()?; let serialised_activity = serde_json::to_string(&activity)?; @@ -232,7 +239,7 @@ where for i in inboxes { let message = SendActivityTask { activity: serialised_activity.to_owned(), - inbox: i, + inbox: i.to_owned(), actor_id: actor.actor_id()?, private_key: actor.private_key().context(location_info!())?, }; diff --git a/lemmy_apub/src/lib.rs b/lemmy_apub/src/lib.rs index bbf1bbbc7..d5d682a7d 100644 --- a/lemmy_apub/src/lib.rs +++ b/lemmy_apub/src/lib.rs @@ -61,13 +61,7 @@ pub static APUB_JSON_CONTENT_TYPE: &str = "application/activity+json"; fn check_is_apub_id_valid(apub_id: &Url) -> Result<(), LemmyError> { let settings = Settings::get(); let domain = apub_id.domain().context(location_info!())?.to_string(); - let local_instance = settings - .hostname - .split(':') - .collect::>() - .first() - .context(location_info!())? - .to_string(); + let local_instance = settings.get_hostname_without_port()?; if !settings.federation.enabled { return if domain == local_instance { diff --git a/lemmy_utils/src/settings.rs b/lemmy_utils/src/settings.rs index 4ca87f282..4877d46b3 100644 --- a/lemmy_utils/src/settings.rs +++ b/lemmy_utils/src/settings.rs @@ -1,3 +1,5 @@ +use crate::location_info; +use anyhow::Context; use config::{Config, ConfigError, Environment, File}; use serde::Deserialize; use std::{env, fs, io::Error, net::IpAddr, path::PathBuf, sync::RwLock}; @@ -178,6 +180,21 @@ impl Settings { format!("{}://{}", self.get_protocol_string(), self.hostname) } + /// When running the federation test setup in `api_tests/` or `docker/federation`, the `hostname` + /// variable will be like `lemmy-alpha:8541`. This method removes the port and returns + /// `lemmy-alpha` instead. It has no effect in production. + pub fn get_hostname_without_port(&self) -> Result { + Ok( + self + .hostname + .split(':') + .collect::>() + .first() + .context(location_info!())? + .to_string(), + ) + } + pub fn save_config_file(data: &str) -> Result { fs::write(CONFIG_FILE, data)?;