From de14636e1073f1c9ac2b9683d230ded891f912fb Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Fri, 12 Mar 2021 16:43:01 +0100 Subject: [PATCH] Fix code to allow sticky/lock from remote moderators --- crates/apub/src/activities/receive/comment.rs | 4 ++-- crates/apub/src/activities/receive/post.rs | 6 ++++-- .../src/activities/receive/private_message.rs | 4 ++-- crates/apub/src/fetcher/community.rs | 2 +- crates/apub/src/fetcher/objects.rs | 10 ++++++++-- crates/apub/src/fetcher/search.rs | 4 ++-- crates/apub/src/fetcher/user.rs | 16 +++++++++++++-- .../apub/src/inbox/receive_for_community.rs | 7 +++++-- crates/apub/src/objects/comment.rs | 5 +++-- crates/apub/src/objects/community.rs | 5 +++-- crates/apub/src/objects/mod.rs | 8 ++++---- crates/apub/src/objects/post.rs | 20 ++++++++++++++++--- crates/apub/src/objects/private_message.rs | 5 +++-- crates/apub/src/objects/user.rs | 5 +++-- 14 files changed, 71 insertions(+), 30 deletions(-) diff --git a/crates/apub/src/activities/receive/comment.rs b/crates/apub/src/activities/receive/comment.rs index 591b6f33a..2b11ad18b 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(¬e, context, user.actor_id(), request_counter).await?; + let comment = Comment::from_apub(¬e, 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(¬e, context, user.actor_id(), request_counter).await?; + let comment = Comment::from_apub(¬e, 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 5bd84ef84..c63706790 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 98c25a588..54379f2ae 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(¬e, context, expected_domain, request_counter).await?; + PrivateMessage::from_apub(¬e, 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(¬e, context, expected_domain, request_counter).await?; + PrivateMessage::from_apub(¬e, 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 9cc1bbd6a..01b30f93b 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 6e0369bde..f2030a066 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::(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 acaccff28..f5ae9dfd4 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 e3ef41c74..e3ea70ea5 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::(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 fed0c4622..0448cccb5 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( activity: &T, announce: &Option, diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs index 6c218190a..3fe90738e 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, request_counter: &mut i32, ) -> Result { let comment: Comment = @@ -126,9 +126,10 @@ impl FromApubToForm for CommentForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result { + let expected_domain = expected_domain.expect("expected_domain must be set for comment"); let creator_actor_id = ¬e .attributed_to() .context(location_info!())? diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index 278bd7b12..efeb7eea5 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, request_counter: &mut i32, ) -> Result { let community: Community = @@ -160,9 +160,10 @@ impl FromApubToForm for CommunityForm { async fn from_apub( group: &GroupExt, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result { + 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 47d80cc23..1dff81026 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, request_counter: &mut i32, ) -> Result where @@ -61,7 +61,7 @@ pub(in crate::objects) trait FromApubToForm { async fn from_apub( apub: &ApubType, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result 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: &From, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result where diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index e9052bdd5..d15f82b70 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, request_counter: &mut i32, ) -> Result { let post: Post = get_object_from_apub(page, context, expected_domain, request_counter).await?; @@ -130,9 +131,17 @@ impl FromApubToForm for PostForm { async fn from_apub( page: &PageExt, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result { + 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 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 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 0bb753e28..0dfa102f4 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, request_counter: &mut i32, ) -> Result { get_object_from_apub(note, context, expected_domain, request_counter).await @@ -87,9 +87,10 @@ impl FromApubToForm for PrivateMessageForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, request_counter: &mut i32, ) -> Result { + 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 83f75e49b..e822fcd92 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, request_counter: &mut i32, ) -> Result { let user_id = person.id_unchecked().context(location_info!())?.to_owned(); @@ -116,9 +116,10 @@ impl FromApubToForm for UserForm { async fn from_apub( person: &PersonExt, _context: &LemmyContext, - expected_domain: Url, + expected_domain: Option, _request_counter: &mut i32, ) -> Result { + 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())?