Dont federate post locking via Update activity (#4717)

* Dont federate post locking via Update activity * cleanup * add missing mod log entries * update assets
LemmyNet · May 15, 2024 · 93c9a5f · 93c9a5f
1 parent 9a9d518
commit 93c9a5f
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 128 deletions.
diff --git a/crates/apub/assets/lemmy/activities/create_or_update/create_page.json b/crates/apub/assets/lemmy/activities/create_or_update/create_page.json
@@ -23,7 +23,6 @@
         "href": "https://lemmy.ml/pictrs/image/xl8W7FZfk9.jpg"
       }
     ],
-    "commentsEnabled": true,
     "sensitive": false,
     "language": {
       "identifier": "ko",

diff --git a/crates/apub/assets/lemmy/activities/create_or_update/update_page.json b/crates/apub/assets/lemmy/activities/create_or_update/update_page.json
@@ -23,7 +23,6 @@
         "href": "https://lemmy.ml/pictrs/image/xl8W7FZfk9.jpg"
       }
     ],
-    "commentsEnabled": true,
     "sensitive": false,
     "published": "2021-10-29T15:10:51.557399Z",
     "updated": "2021-10-29T15:11:35.976374Z"

diff --git a/crates/apub/assets/lemmy/collections/group_featured_posts.json b/crates/apub/assets/lemmy/collections/group_featured_posts.json
@@ -15,7 +15,6 @@
       "cc": [],
       "mediaType": "text/html",
       "attachment": [],
-      "commentsEnabled": true,
       "sensitive": false,
       "published": "2023-02-06T06:42:41.939437Z",
       "language": {
@@ -36,7 +35,6 @@
       "cc": [],
       "mediaType": "text/html",
       "attachment": [],
-      "commentsEnabled": true,
       "sensitive": false,
       "published": "2023-02-06T06:42:37.119567Z",
       "language": {

diff --git a/crates/apub/assets/lemmy/collections/group_outbox.json b/crates/apub/assets/lemmy/collections/group_outbox.json
@@ -22,7 +22,6 @@
           ],
           "name": "another outbox test",
           "mediaType": "text/html",
-          "commentsEnabled": true,
           "sensitive": false,
           "stickied": false,
           "published": "2021-11-18T17:19:45.895163Z"
@@ -51,7 +50,6 @@
           ],
           "name": "outbox test",
           "mediaType": "text/html",
-          "commentsEnabled": true,
           "sensitive": false,
           "stickied": false,
           "published": "2021-11-18T17:19:05.763109Z"

diff --git a/crates/apub/assets/lemmy/objects/page.json b/crates/apub/assets/lemmy/objects/page.json
@@ -25,7 +25,6 @@
     "url": "https://enterprise.lemmy.ml/pictrs/image/eOtYb9iEiB.png"
   },
   "sensitive": false,
-  "commentsEnabled": true,
   "language": {
     "identifier": "fr",
     "name": "Français"

diff --git a/crates/apub/src/activities/community/lock_page.rs b/crates/apub/src/activities/community/lock_page.rs
@@ -26,6 +26,7 @@ use lemmy_db_schema::{
   source::{
     activity::ActivitySendTargets,
     community::Community,
+    moderator::{ModLockPost, ModLockPostForm},
     person::Person,
     post::{Post, PostUpdateForm},
   },
@@ -60,12 +61,22 @@ impl ActivityHandler for LockPage {
   }
 
   async fn receive(self, context: &Data<Self::DataType>) -> Result<(), Self::Error> {
+    insert_received_activity(&self.id, context).await?;
+    let locked = Some(true);
     let form = PostUpdateForm {
-      locked: Some(true),
+      locked,
       ..Default::default()
     };
     let post = self.object.dereference(context).await?;
     Post::update(&mut context.pool(), post.id, &form).await?;
+
+    let form = ModLockPostForm {
+      mod_person_id: self.actor.dereference(context).await?.id,
+      post_id: post.id,
+      locked,
+    };
+    ModLockPost::create(&mut context.pool(), &form).await?;
+
     Ok(())
   }
 }
@@ -94,12 +105,21 @@ impl ActivityHandler for UndoLockPage {
 
   async fn receive(self, context: &Data<Self::DataType>) -> Result<(), Self::Error> {
     insert_received_activity(&self.id, context).await?;
+    let locked = Some(false);
     let form = PostUpdateForm {
-      locked: Some(false),
+      locked,
       ..Default::default()
     };
     let post = self.object.object.dereference(context).await?;
     Post::update(&mut context.pool(), post.id, &form).await?;
+
+    let form = ModLockPostForm {
+      mod_person_id: self.actor.dereference(context).await?.id,
+      post_id: post.id,
+      locked,
+    };
+    ModLockPost::create(&mut context.pool(), &form).await?;
+
     Ok(())
   }
 }

diff --git a/crates/apub/src/activities/create_or_update/post.rs b/crates/apub/src/activities/create_or_update/post.rs
@@ -4,7 +4,6 @@ use crate::{
     community::send_activity_in_community,
     generate_activity_id,
     verify_is_public,
-    verify_mod_action,
     verify_person_in_community,
   },
   activity_lists::AnnouncableActivities,
@@ -78,14 +77,13 @@ impl CreateOrUpdatePage {
 
     let create_or_update =
       CreateOrUpdatePage::new(post.into(), &person, &community, kind, &context).await?;
-    let is_mod_action = create_or_update.object.is_mod_action(&context).await?;
     let activity = AnnouncableActivities::CreateOrUpdatePost(create_or_update);
     send_activity_in_community(
       activity,
       &person,
       &community,
       ActivitySendTargets::empty(),
-      is_mod_action,
+      false,
       &context,
     )
     .await?;
@@ -112,30 +110,8 @@ impl ActivityHandler for CreateOrUpdatePage {
     let community = self.community(context).await?;
     verify_person_in_community(&self.actor, &community, context).await?;
     check_community_deleted_or_removed(&community)?;
-
-    match self.kind {
-      CreateOrUpdateType::Create => {
-        verify_domains_match(self.actor.inner(), self.object.id.inner())?;
-        verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?;
-        // Check that the post isnt locked, as that isnt possible for newly created posts.
-        // However, when fetching a remote post we generate a new create activity with the current
-        // locked value, so this check may fail. So only check if its a local community,
-        // because then we will definitely receive all create and update activities separately.
-        let is_locked = self.object.comments_enabled == Some(false);
-        if community.local && is_locked {
-          Err(LemmyErrorType::NewPostCannotBeLocked)?
-        }
-      }
-      CreateOrUpdateType::Update => {
-        let is_mod_action = self.object.is_mod_action(context).await?;
-        if is_mod_action {
-          verify_mod_action(&self.actor, &community, context).await?;
-        } else {
-          verify_domains_match(self.actor.inner(), self.object.id.inner())?;
-          verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?;
-        }
-      }
-    }
+    verify_domains_match(self.actor.inner(), self.object.id.inner())?;
+    verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?;
     ApubPost::verify(&self.object, self.actor.inner(), context).await?;
     Ok(())
   }

diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs
@@ -36,7 +36,6 @@ use lemmy_db_schema::{
   source::{
     community::Community,
     local_site::LocalSite,
-    moderator::{ModLockPost, ModLockPostForm},
     person::Person,
     post::{Post, PostInsertForm, PostUpdateForm},
   },
@@ -147,7 +146,6 @@ impl Object for ApubPost {
       source: self.body.clone().map(Source::new),
       attachment,
       image: self.thumbnail_url.clone().map(ImageObject::new),
-      comments_enabled: Some(!self.locked),
       sensitive: Some(self.nsfw),
       language,
       published: Some(self.published),
@@ -165,12 +163,8 @@ impl Object for ApubPost {
     expected_domain: &Url,
     context: &Data<Self::DataType>,
   ) -> LemmyResult<()> {
-    // We can't verify the domain in case of mod action, because the mod may be on a different
-    // 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, context)?;
-    };
+    verify_domains_match(page.id.inner(), expected_domain)?;
+    verify_is_remote_object(&page.id, context)?;
 
     let community = page.community(context).await?;
     check_apub_id_valid_with_strictness(page.id.inner(), community.local, context).await?;
@@ -218,62 +212,46 @@ impl Object for ApubPost {
       name = name.chars().take(MAX_TITLE_LENGTH).collect();
     }
 
-    // read existing, local post if any (for generating mod log)
-    let old_post = page.id.dereference_local(context).await;
-
     let first_attachment = page.attachment.first();
     let local_site = LocalSite::read(&mut context.pool()).await.ok();
 
-    let form = if !page.is_mod_action(context).await? {
-      let url = if let Some(attachment) = first_attachment.cloned() {
-        Some(attachment.url())
-      } else if page.kind == PageType::Video {
-        // we cant display videos directly, so insert a link to external video page
-        Some(page.id.inner().clone())
-      } else {
-        None
-      };
-      check_url_scheme(&url)?;
-
-      let alt_text = first_attachment.cloned().and_then(Attachment::alt_text);
-
-      let url = proxy_image_link_opt_apub(url, context).await?;
-
-      let slur_regex = &local_site_opt_to_slur_regex(&local_site);
-      let url_blocklist = get_url_blocklist(context).await?;
-
-      let body = read_from_string_or_source_opt(&page.content, &page.media_type, &page.source);
-      let body = process_markdown_opt(&body, slur_regex, &url_blocklist, context).await?;
-      let language_id =
-        LanguageTag::to_language_id_single(page.language, &mut context.pool()).await?;
-
-      PostInsertForm::builder()
-        .name(name)
-        .url(url.map(Into::into))
-        .body(body)
-        .alt_text(alt_text)
-        .creator_id(creator.id)
-        .community_id(community.id)
-        .locked(page.comments_enabled.map(|e| !e))
-        .published(page.published.map(Into::into))
-        .updated(page.updated.map(Into::into))
-        .deleted(Some(false))
-        .nsfw(page.sensitive)
-        .ap_id(Some(page.id.clone().into()))
-        .local(Some(false))
-        .language_id(language_id)
-        .build()
+    let url = if let Some(attachment) = first_attachment.cloned() {
+      Some(attachment.url())
+    } else if page.kind == PageType::Video {
+      // we cant display videos directly, so insert a link to external video page
+      Some(page.id.inner().clone())
     } else {
-      // if is mod action, only update locked/stickied fields, nothing else
-      PostInsertForm::builder()
-        .name(name)
-        .creator_id(creator.id)
-        .community_id(community.id)
-        .ap_id(Some(page.id.clone().into()))
-        .locked(page.comments_enabled.map(|e| !e))
-        .updated(page.updated.map(Into::into))
-        .build()
+      None
     };
+    check_url_scheme(&url)?;
+
+    let alt_text = first_attachment.cloned().and_then(Attachment::alt_text);
+
+    let url = proxy_image_link_opt_apub(url, context).await?;
+
+    let slur_regex = &local_site_opt_to_slur_regex(&local_site);
+    let url_blocklist = get_url_blocklist(context).await?;
+
+    let body = read_from_string_or_source_opt(&page.content, &page.media_type, &page.source);
+    let body = process_markdown_opt(&body, slur_regex, &url_blocklist, context).await?;
+    let language_id =
+      LanguageTag::to_language_id_single(page.language, &mut context.pool()).await?;
+
+    let form = PostInsertForm::builder()
+      .name(name)
+      .url(url.map(Into::into))
+      .body(body)
+      .alt_text(alt_text)
+      .creator_id(creator.id)
+      .community_id(community.id)
+      .published(page.published.map(Into::into))
+      .updated(page.updated.map(Into::into))
+      .deleted(Some(false))
+      .nsfw(page.sensitive)
+      .ap_id(Some(page.id.clone().into()))
+      .local(Some(false))
+      .language_id(language_id)
+      .build();
 
     let timestamp = page.updated.or(page.published).unwrap_or_else(naive_now);
     let post = Post::insert_apub(&mut context.pool(), timestamp, &form).await?;
@@ -287,16 +265,6 @@ impl Object for ApubPost {
       context.reset_request_count(),
     );
 
-    // write mod log entry for lock
-    if Page::is_locked_changed(&old_post, &page.comments_enabled) {
-      let form = ModLockPostForm {
-        mod_person_id: creator.id,
-        post_id: post.id,
-        locked: Some(post.locked),
-      };
-      ModLockPost::create(&mut context.pool(), &form).await?;
-    }
-
     Ok(post.into())
   }
 }

diff --git a/crates/apub/src/protocol/objects/page.rs b/crates/apub/src/protocol/objects/page.rs
@@ -60,7 +60,6 @@ pub struct Page {
   #[serde(default)]
   pub(crate) attachment: Vec<Attachment>,
   pub(crate) image: Option<ImageObject>,
-  pub(crate) comments_enabled: Option<bool>,
   pub(crate) sensitive: Option<bool>,
   pub(crate) published: Option<DateTime<Utc>>,
   pub(crate) updated: Option<DateTime<Utc>>,
@@ -156,28 +155,6 @@ pub enum HashtagType {
 }
 
 impl Page {
-  /// Only mods can change the post's locked status. So if it is changed from the default value,
-  /// it is a mod action and needs to be verified as such.
-  ///
-  /// Locked needs to be false on a newly created post (verified in [[CreatePost]].
-  pub(crate) async fn is_mod_action(&self, context: &Data<LemmyContext>) -> LemmyResult<bool> {
-    let old_post = self.id.clone().dereference_local(context).await;
-    Ok(Page::is_locked_changed(&old_post, &self.comments_enabled))
-  }
-
-  pub(crate) fn is_locked_changed<E>(
-    old_post: &Result<ApubPost, E>,
-    new_comments_enabled: &Option<bool>,
-  ) -> bool {
-    if let Some(new_comments_enabled) = new_comments_enabled {
-      if let Ok(old_post) = old_post {
-        return new_comments_enabled != &!old_post.locked;
-      }
-    }
-
-    false
-  }
-
   pub(crate) fn creator(&self) -> LemmyResult<ObjectId<ApubPerson>> {
     match &self.attributed_to {
       AttributedTo::Lemmy(l) => Ok(l.clone()),