From 93c9a5f2b143dc6ced672b820dc65ed6e705b896 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 15 May 2024 13:36:00 +0200 Subject: [PATCH] Dont federate post locking via Update activity (#4717) * Dont federate post locking via Update activity * cleanup * add missing mod log entries * update assets --- .../create_or_update/create_page.json | 1 - .../create_or_update/update_page.json | 1 - .../collections/group_featured_posts.json | 2 - .../lemmy/collections/group_outbox.json | 2 - crates/apub/assets/lemmy/objects/page.json | 1 - .../src/activities/community/lock_page.rs | 24 +++- .../src/activities/create_or_update/post.rs | 30 +---- crates/apub/src/objects/post.rs | 106 ++++++------------ crates/apub/src/protocol/objects/page.rs | 23 ---- 9 files changed, 62 insertions(+), 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 index 50d2536fe..114c5b557 100644 --- 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 index 888b866b8..e9569e6f7 100644 --- 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 index 59f1afb9c..70b4d7d3a 100644 --- 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 index c7279a799..231399e13 100644 --- 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 index 6b536dd90..20af5dfd2 100644 --- 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 index bafb42a4a..322cd88c2 100644 --- 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) -> 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) -> 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 index 53900c799..e8bfc36e6 100644 --- 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 index ff11c985c..929d598cd 100644 --- 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, ) -> 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 index bbb46bfaf..6075b14a1 100644 --- 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, pub(crate) image: Option, - pub(crate) comments_enabled: Option, pub(crate) sensitive: Option, pub(crate) published: Option>, pub(crate) updated: Option>, @@ -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) -> LemmyResult { - 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( - old_post: &Result, - new_comments_enabled: &Option, - ) -> 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> { match &self.attributed_to { AttributedTo::Lemmy(l) => Ok(l.clone()),