Federation: dont overwrite local object from Announce activity (#2232)

* Federation: dont overwrite local object from Announce activity (fixes #2143)

* add missing form fields

* refactoring

* add ap_id, updated fields

* fix

crates/apub/src/objects/comment.rs

@@ -2,7 +2,7 @@ use crate::{
   activities::{verify_is_public, verify_person_in_community},
   check_is_apub_id_valid,
   mentions::collect_non_local_mentions,
-  objects::read_from_string_or_source,
+  objects::{read_from_string_or_source, verify_is_remote_object},
   protocol::{
     objects::{note::Note, tombstone::Tombstone},
     Source,
@@ -149,6 +149,7 @@ impl ApubObject for ApubComment {
     })
     .await??;
     check_is_apub_id_valid(note.id.inner(), community.local, &context.settings())?;
+    verify_is_remote_object(note.id.inner())?;
     verify_person_in_community(
       &note.attributed_to,
       &community.into(),

crates/apub/src/objects/mod.rs

@@ -1,7 +1,8 @@
 use crate::protocol::{ImageObject, Source};
+use anyhow::anyhow;
 use html2md::parse_html;
 use lemmy_apub_lib::verify::verify_domains_match;
-use lemmy_utils::LemmyError;
+use lemmy_utils::{settings::structs::Settings, LemmyError};
 use url::Url;
 
 pub mod comment;
@@ -30,7 +31,10 @@ pub(crate) fn read_from_string_or_source_opt(
   }
 }
 
-pub fn verify_image_domain_matches(a: &Url, b: &Option<ImageObject>) -> Result<(), LemmyError> {
+pub(crate) fn verify_image_domain_matches(
+  a: &Url,
+  b: &Option<ImageObject>,
+) -> Result<(), LemmyError> {
   if let Some(b) = b {
     verify_domains_match(a, &b.url)
   } else {
@@ -38,6 +42,19 @@ pub fn verify_image_domain_matches(a: &Url, b: &Option<ImageObject>) -> Result<(
   }
 }
 
+/// When for example a Post is made in a remote community, the community will send it back,
+/// wrapped in Announce. If we simply receive this like any other federated object, overwrite the
+/// existing, local Post. In particular, it will set the field local = false, so that the object
+/// can't be fetched from the Activitypub HTTP endpoint anymore (which only serves local objects).
+pub(crate) fn verify_is_remote_object(id: &Url) -> Result<(), LemmyError> {
+  let local_domain = Settings::get().get_hostname_without_port()?;
+  if id.domain() == Some(&local_domain) {
+    Err(anyhow!("cant accept local object from remote instance").into())
+  } else {
+    Ok(())
+  }
+}
+
 #[cfg(test)]
 pub(crate) mod tests {
   use actix::Actor;

crates/apub/src/objects/post.rs

@@ -1,7 +1,7 @@
 use crate::{
   activities::{verify_is_public, verify_person_in_community},
   check_is_apub_id_valid,
-  objects::read_from_string_or_source_opt,
+  objects::{read_from_string_or_source_opt, verify_is_remote_object},
   protocol::{
     objects::{
       page::{Attachment, Page, PageType},
@@ -139,6 +139,7 @@ impl ApubObject for ApubPost {
     // instance from the post author.
     if !page.is_mod_action(context).await? {
       verify_domains_match(page.id.inner(), expected_domain)?;
+      verify_is_remote_object(page.id.inner())?;
     };
 
     let community = page.extract_community(context, request_counter).await?;
@@ -162,42 +163,56 @@ impl ApubObject for ApubPost {
       .await?;
     let community = page.extract_community(context, request_counter).await?;
 
-    let url = if let Some(attachment) = page.attachment.first() {
+    let form = if !page.is_mod_action(context).await? {
-      Some(attachment.href.clone())
+      let url = if let Some(attachment) = page.attachment.first() {
-    } else {
+        Some(attachment.href.clone())
-      page.url
+      } else {
-    };
+        page.url
-    let thumbnail_url: Option<Url> = page.image.map(|i| i.url);
+      };
-    let (metadata_res, pictrs_thumbnail) = if let Some(url) = &url {
+      let thumbnail_url: Option<Url> = page.image.map(|i| i.url);
-      fetch_site_data(context.client(), &context.settings(), Some(url)).await
+      let (metadata_res, pictrs_thumbnail) = if let Some(url) = &url {
-    } else {
+        fetch_site_data(context.client(), &context.settings(), Some(url)).await
-      (None, thumbnail_url)
+      } else {
-    };
+        (None, thumbnail_url)
-    let (embed_title, embed_description, embed_html) = metadata_res
+      };
-      .map(|u| (u.title, u.description, u.html))
+      let (embed_title, embed_description, embed_html) = metadata_res
-      .unwrap_or((None, None, None));
+        .map(|u| (u.title, u.description, u.html))
-    let body_slurs_removed = read_from_string_or_source_opt(&page.content, &page.source)
+        .unwrap_or((None, None, None));
-      .map(|s| remove_slurs(&s, &context.settings().slur_regex()));
+      let body_slurs_removed = read_from_string_or_source_opt(&page.content, &page.source)
+        .map(|s| remove_slurs(&s, &context.settings().slur_regex()));
 
-    let form = PostForm {
+      PostForm {
-      name: page.name.clone(),
+        name: page.name.clone(),
-      url: url.map(Into::into),
+        url: url.map(Into::into),
-      body: body_slurs_removed,
+        body: body_slurs_removed,
-      creator_id: creator.id,
+        creator_id: creator.id,
-      community_id: community.id,
+        community_id: community.id,
-      removed: None,
+        removed: None,
-      locked: page.comments_enabled.map(|e| !e),
+        locked: page.comments_enabled.map(|e| !e),
-      published: page.published.map(|u| u.naive_local()),
+        published: page.published.map(|u| u.naive_local()),
-      updated: page.updated.map(|u| u.naive_local()),
+        updated: page.updated.map(|u| u.naive_local()),
-      deleted: None,
+        deleted: None,
-      nsfw: page.sensitive,
+        nsfw: page.sensitive,
-      stickied: page.stickied,
+        stickied: page.stickied,
-      embed_title,
+        embed_title,
-      embed_description,
+        embed_description,
-      embed_html,
+        embed_html,
-      thumbnail_url: pictrs_thumbnail.map(|u| u.into()),
+        thumbnail_url: pictrs_thumbnail.map(|u| u.into()),
-      ap_id: Some(page.id.clone().into()),
+        ap_id: Some(page.id.clone().into()),
-      local: Some(false),
+        local: Some(false),
+      }
+    } else {
+      // if is mod action, only update locked/stickied fields, nothing else
+      PostForm {
+        name: page.name.clone(),
+        creator_id: creator.id,
+        community_id: community.id,
+        locked: page.comments_enabled.map(|e| !e),
+        stickied: page.stickied,
+        updated: page.updated.map(|u| u.naive_local()),
+        ap_id: Some(page.id.clone().into()),
+        ..Default::default()
+      }
     };
 
     // read existing, local post if any (for generating mod log)

crates/apub/src/protocol/objects/page.rs

@@ -75,9 +75,9 @@ impl Page {
       .dereference_local(context)
       .await;
 
-    let is_mod_action = Page::is_stickied_changed(&old_post, &self.stickied)
+    let stickied_changed = Page::is_stickied_changed(&old_post, &self.stickied);
-      || Page::is_locked_changed(&old_post, &self.comments_enabled);
+    let locked_changed = Page::is_locked_changed(&old_post, &self.comments_enabled);
-    Ok(is_mod_action)
+    Ok(stickied_changed || locked_changed)
   }
 
   pub(crate) fn is_stickied_changed<E>(