From: Felix Ableitner <me@nutomic.com>
Date: Fri, 12 Mar 2021 15:43:01 +0000 (+0100)
Subject: Fix code to allow sticky/lock from remote moderators
X-Git-Url: http://these/git/%7B%60%24%7BwebArchiveUrl%7D/%22%7B%7D/%24%7B%60data:application/%22https:/hacktivis.me/%7Bthis.imageSrc.unwrap%28%29%7D?a=commitdiff_plain;h=de14636e1073f1c9ac2b9683d230ded891f912fb;p=lemmy.git

Fix code to allow sticky/lock from remote moderators
---

diff --git a/crates/apub/src/activities/receive/comment.rs b/crates/apub/src/activities/receive/comment.rs
index 591b6f33..2b11ad18 100644
--- a/crates/apub/src/activities/receive/comment.rs
+++ b/crates/apub/src/activities/receive/comment.rs
@@ -23,7 +23,7 @@ pub(crate) async fn receive_create_comment(
   let note = NoteExt::from_any_base(create.object().to_owned().one().context(location_info!())?)?
     .context(location_info!())?;
 
-  let comment = Comment::from_apub(&note, context, user.actor_id(), request_counter).await?;
+  let comment = Comment::from_apub(&note, context, Some(user.actor_id()), request_counter).await?;
 
   let post_id = comment.post_id;
   let post = blocking(context.pool(), move |conn| Post::read(conn, post_id)).await??;
@@ -66,7 +66,7 @@ pub(crate) async fn receive_update_comment(
     .context(location_info!())?;
   let user = get_actor_as_user(&update, context, request_counter).await?;
 
-  let comment = Comment::from_apub(&note, context, user.actor_id(), request_counter).await?;
+  let comment = Comment::from_apub(&note, context, Some(user.actor_id()), request_counter).await?;
 
   let comment_id = comment.id;
   let post_id = comment.post_id;
diff --git a/crates/apub/src/activities/receive/post.rs b/crates/apub/src/activities/receive/post.rs
index 5bd84ef8..c6370679 100644
--- a/crates/apub/src/activities/receive/post.rs
+++ b/crates/apub/src/activities/receive/post.rs
@@ -32,7 +32,7 @@ pub(crate) async fn receive_create_post(
   let page = PageExt::from_any_base(create.object().to_owned().one().context(location_info!())?)?
     .context(location_info!())?;
 
-  let post = Post::from_apub(&page, context, user.actor_id(), request_counter).await?;
+  let post = Post::from_apub(&page, context, Some(user.actor_id()), request_counter).await?;
 
   // Refetch the view
   let post_id = post.id;
@@ -72,6 +72,7 @@ pub(crate) async fn receive_update_post(
   })
   .await??;
 
+  let mut expected_domain = Some(user.actor_id());
   // If sticked or locked state was changed, make sure the actor is a mod
   let stickied = page.ext_one.stickied.context(location_info!())?;
   let locked = !page.ext_one.comments_enabled.context(location_info!())?;
@@ -81,9 +82,10 @@ pub(crate) async fn receive_update_post(
     })
     .await??;
     verify_mod_activity(&update, announce, &community, context).await?;
+    expected_domain = None;
   }
 
-  let post = Post::from_apub(&page, context, user.actor_id(), request_counter).await?;
+  let post = Post::from_apub(&page, context, expected_domain, request_counter).await?;
 
   let post_id = post.id;
   // Refetch the view
diff --git a/crates/apub/src/activities/receive/private_message.rs b/crates/apub/src/activities/receive/private_message.rs
index 98c25a58..54379f2a 100644
--- a/crates/apub/src/activities/receive/private_message.rs
+++ b/crates/apub/src/activities/receive/private_message.rs
@@ -39,7 +39,7 @@ pub(crate) async fn receive_create_private_message(
   .context(location_info!())?;
 
   let private_message =
-    PrivateMessage::from_apub(&note, context, expected_domain, request_counter).await?;
+    PrivateMessage::from_apub(&note, context, Some(expected_domain), request_counter).await?;
 
   let message = blocking(&context.pool(), move |conn| {
     PrivateMessageView::read(conn, private_message.id)
@@ -78,7 +78,7 @@ pub(crate) async fn receive_update_private_message(
   let note = NoteExt::from_any_base(object)?.context(location_info!())?;
 
   let private_message =
-    PrivateMessage::from_apub(&note, context, expected_domain, request_counter).await?;
+    PrivateMessage::from_apub(&note, context, Some(expected_domain), request_counter).await?;
 
   let private_message_id = private_message.id;
   let message = blocking(&context.pool(), move |conn| {
diff --git a/crates/apub/src/fetcher/community.rs b/crates/apub/src/fetcher/community.rs
index 9cc1bbd6..01b30f93 100644
--- a/crates/apub/src/fetcher/community.rs
+++ b/crates/apub/src/fetcher/community.rs
@@ -72,7 +72,7 @@ async fn fetch_remote_community(
 
   let group = group?;
   let community =
-    Community::from_apub(&group, context, apub_id.to_owned(), recursion_counter).await?;
+    Community::from_apub(&group, context, Some(apub_id.to_owned()), recursion_counter).await?;
 
   // only fetch outbox for new communities, otherwise this can create an infinite loop
   if old_community.is_none() {
diff --git a/crates/apub/src/fetcher/objects.rs b/crates/apub/src/fetcher/objects.rs
index 6e0369bd..f2030a06 100644
--- a/crates/apub/src/fetcher/objects.rs
+++ b/crates/apub/src/fetcher/objects.rs
@@ -30,7 +30,13 @@ pub(crate) async fn get_or_fetch_and_insert_post(
       debug!("Fetching and creating remote post: {}", post_ap_id);
       let page =
         fetch_remote_object::<PageExt>(context.client(), post_ap_id, recursion_counter).await?;
-      let post = Post::from_apub(&page, context, post_ap_id.to_owned(), recursion_counter).await?;
+      let post = Post::from_apub(
+        &page,
+        context,
+        Some(post_ap_id.to_owned()),
+        recursion_counter,
+      )
+      .await?;
 
       Ok(post)
     }
@@ -65,7 +71,7 @@ pub(crate) async fn get_or_fetch_and_insert_comment(
       let comment = Comment::from_apub(
         &comment,
         context,
-        comment_ap_id.to_owned(),
+        Some(comment_ap_id.to_owned()),
         recursion_counter,
       )
       .await?;
diff --git a/crates/apub/src/fetcher/search.rs b/crates/apub/src/fetcher/search.rs
index acaccff2..f5ae9dfd 100644
--- a/crates/apub/src/fetcher/search.rs
+++ b/crates/apub/src/fetcher/search.rs
@@ -147,13 +147,13 @@ async fn build_response(
       ];
     }
     SearchAcceptedObjects::Page(p) => {
-      let p = Post::from_apub(&p, context, query_url, recursion_counter).await?;
+      let p = Post::from_apub(&p, context, Some(query_url), recursion_counter).await?;
 
       response.posts =
         vec![blocking(context.pool(), move |conn| PostView::read(conn, p.id, None)).await??];
     }
     SearchAcceptedObjects::Comment(c) => {
-      let c = Comment::from_apub(&c, context, query_url, recursion_counter).await?;
+      let c = Comment::from_apub(&c, context, Some(query_url), recursion_counter).await?;
 
       response.comments = vec![
         blocking(context.pool(), move |conn| {
diff --git a/crates/apub/src/fetcher/user.rs b/crates/apub/src/fetcher/user.rs
index e3ef41c7..e3ea70ea 100644
--- a/crates/apub/src/fetcher/user.rs
+++ b/crates/apub/src/fetcher/user.rs
@@ -46,7 +46,13 @@ pub(crate) async fn get_or_fetch_and_upsert_user(
         return Ok(u);
       }
 
-      let user = User_::from_apub(&person?, context, apub_id.to_owned(), recursion_counter).await?;
+      let user = User_::from_apub(
+        &person?,
+        context,
+        Some(apub_id.to_owned()),
+        recursion_counter,
+      )
+      .await?;
 
       let user_id = user.id;
       blocking(context.pool(), move |conn| {
@@ -62,7 +68,13 @@ pub(crate) async fn get_or_fetch_and_upsert_user(
       let person =
         fetch_remote_object::<PersonExt>(context.client(), apub_id, recursion_counter).await?;
 
-      let user = User_::from_apub(&person, context, apub_id.to_owned(), recursion_counter).await?;
+      let user = User_::from_apub(
+        &person,
+        context,
+        Some(apub_id.to_owned()),
+        recursion_counter,
+      )
+      .await?;
 
       Ok(user)
     }
diff --git a/crates/apub/src/inbox/receive_for_community.rs b/crates/apub/src/inbox/receive_for_community.rs
index fed0c462..0448cccb 100644
--- a/crates/apub/src/inbox/receive_for_community.rs
+++ b/crates/apub/src/inbox/receive_for_community.rs
@@ -117,7 +117,7 @@ pub(in crate::inbox) async fn receive_update_for_community(
   request_counter: &mut i32,
 ) -> Result<(), LemmyError> {
   let update = Update::from_any_base(activity)?.context(location_info!())?;
-  verify_activity_domains_valid(&update, &expected_domain, true)?;
+  verify_activity_domains_valid(&update, &expected_domain, false)?;
   verify_is_addressed_to_public(&update)?;
   verify_modification_actor_instance(&update, &announce, context).await?;
 
@@ -402,7 +402,7 @@ pub(in crate::inbox) async fn receive_add_for_community(
     CommunityModerator::get_user_moderated_communities(conn, new_mod_id)
   })
   .await??;
-  if moderated_communities.contains(&community.id) {
+  if !moderated_communities.contains(&community.id) {
     let form = CommunityModeratorForm {
       community_id: community.id,
       user_id: new_mod.id,
@@ -575,6 +575,9 @@ where
 
 /// For activities like Update, Delete or Undo, check that the actor is from the same instance
 /// as the original object itself (or is a remote mod).
+///
+/// Note: This is only needed for mod actions. Normal user actions (edit post, undo vote etc) are
+///       already verified with `expected_domain`, so this serves as an additional check.
 async fn verify_modification_actor_instance<T, Kind>(
   activity: &T,
   announce: &Option<Announce>,
diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs
index 6c218190..3fe90738 100644
--- a/crates/apub/src/objects/comment.rs
+++ b/crates/apub/src/objects/comment.rs
@@ -97,7 +97,7 @@ impl FromApub for Comment {
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Comment, LemmyError> {
     let comment: Comment =
@@ -126,9 +126,10 @@ impl FromApubToForm<NoteExt> for CommentForm {
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<CommentForm, LemmyError> {
+    let expected_domain = expected_domain.expect("expected_domain must be set for comment");
     let creator_actor_id = &note
       .attributed_to()
       .context(location_info!())?
diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs
index 278bd7b1..efeb7eea 100644
--- a/crates/apub/src/objects/community.rs
+++ b/crates/apub/src/objects/community.rs
@@ -105,7 +105,7 @@ impl FromApub for Community {
   async fn from_apub(
     group: &GroupExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Community, LemmyError> {
     let community: Community =
@@ -160,9 +160,10 @@ impl FromApubToForm<GroupExt> for CommunityForm {
   async fn from_apub(
     group: &GroupExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Self, LemmyError> {
+    let expected_domain = expected_domain.expect("expected_domain must be set for community");
     let moderator_uris = fetch_community_mods(context, group, request_counter).await?;
     let creator_uri = moderator_uris.first().context(location_info!())?;
 
diff --git a/crates/apub/src/objects/mod.rs b/crates/apub/src/objects/mod.rs
index 47d80cc2..1dff8102 100644
--- a/crates/apub/src/objects/mod.rs
+++ b/crates/apub/src/objects/mod.rs
@@ -45,11 +45,11 @@ pub(crate) trait FromApub {
   ///
   /// * `apub` The object to read from
   /// * `context` LemmyContext which holds DB pool, HTTP client etc
-  /// * `expected_domain` Domain where the object was received from
+  /// * `expected_domain` Domain where the object was received from. None in case of mod action.
   async fn from_apub(
     apub: &Self::ApubType,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Self, LemmyError>
   where
@@ -61,7 +61,7 @@ pub(in crate::objects) trait FromApubToForm<ApubType> {
   async fn from_apub(
     apub: &ApubType,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Self, LemmyError>
   where
@@ -173,7 +173,7 @@ pub(in crate::objects) fn check_is_markdown(mime: Option<&Mime>) -> Result<(), L
 pub(in crate::objects) async fn get_object_from_apub<From, Kind, To, ToForm>(
   from: &From,
   context: &LemmyContext,
-  expected_domain: Url,
+  expected_domain: Option<Url>,
   request_counter: &mut i32,
 ) -> Result<To, LemmyError>
 where
diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs
index e9052bdd..d15f82b7 100644
--- a/crates/apub/src/objects/post.rs
+++ b/crates/apub/src/objects/post.rs
@@ -1,4 +1,5 @@
 use crate::{
+  check_is_apub_id_valid,
   extensions::{context::lemmy_context, page_extension::PageExtension},
   fetcher::user::get_or_fetch_and_upsert_user,
   objects::{
@@ -115,7 +116,7 @@ impl FromApub for Post {
   async fn from_apub(
     page: &PageExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<Post, LemmyError> {
     let post: Post = get_object_from_apub(page, context, expected_domain, request_counter).await?;
@@ -130,9 +131,17 @@ impl FromApubToForm<PageExt> for PostForm {
   async fn from_apub(
     page: &PageExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<PostForm, LemmyError> {
+    let ap_id = match expected_domain {
+      Some(e) => check_object_domain(page, e)?,
+      None => {
+        let id = page.id_unchecked().context(location_info!())?;
+        check_is_apub_id_valid(id)?;
+        id.to_owned().into()
+      }
+    };
     let ext = &page.ext_one;
     let creator_actor_id = page
       .inner
@@ -187,6 +196,11 @@ impl FromApubToForm<PageExt> for PostForm {
       .to_string();
     let body = get_source_markdown_value(page)?;
 
+    // TODO: expected_domain is wrong in this case, because it simply takes the domain of the actor
+    //       maybe we need to take id_unchecked() if the activity is from community to user?
+    //       why did this work before? -> i dont think it did?
+    //       -> try to make expected_domain optional and set it null if it is a mod action
+
     check_slurs(&name)?;
     let body_slurs_removed = body.map(|b| remove_slurs(&b));
     Ok(PostForm {
@@ -214,7 +228,7 @@ impl FromApubToForm<PageExt> for PostForm {
       embed_description: iframely_description,
       embed_html: iframely_html,
       thumbnail_url: pictrs_thumbnail.map(|u| u.into()),
-      ap_id: Some(check_object_domain(page, expected_domain)?),
+      ap_id: Some(ap_id),
       local: false,
     })
   }
diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs
index 0bb753e2..0dfa102f 100644
--- a/crates/apub/src/objects/private_message.rs
+++ b/crates/apub/src/objects/private_message.rs
@@ -75,7 +75,7 @@ impl FromApub for PrivateMessage {
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<PrivateMessage, LemmyError> {
     get_object_from_apub(note, context, expected_domain, request_counter).await
@@ -87,9 +87,10 @@ impl FromApubToForm<NoteExt> for PrivateMessageForm {
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<PrivateMessageForm, LemmyError> {
+    let expected_domain = expected_domain.expect("expected_domain must be set for private message");
     let creator_actor_id = note
       .attributed_to()
       .context(location_info!())?
diff --git a/crates/apub/src/objects/user.rs b/crates/apub/src/objects/user.rs
index 83f75e49..e822fcd9 100644
--- a/crates/apub/src/objects/user.rs
+++ b/crates/apub/src/objects/user.rs
@@ -91,7 +91,7 @@ impl FromApub for User_ {
   async fn from_apub(
     person: &PersonExt,
     context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     request_counter: &mut i32,
   ) -> Result<User_, LemmyError> {
     let user_id = person.id_unchecked().context(location_info!())?.to_owned();
@@ -116,9 +116,10 @@ impl FromApubToForm<PersonExt> for UserForm {
   async fn from_apub(
     person: &PersonExt,
     _context: &LemmyContext,
-    expected_domain: Url,
+    expected_domain: Option<Url>,
     _request_counter: &mut i32,
   ) -> Result<Self, LemmyError> {
+    let expected_domain = expected_domain.expect("expected_domain must be set for user");
     let avatar = match person.icon() {
       Some(any_image) => Some(
         Image::from_any_base(any_image.as_one().context(location_info!())?.clone())?