From 429b7f6e5c61752e679a55b9b67d8bad9894b9b9 Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Thu, 3 Dec 2020 14:35:34 +0100 Subject: [PATCH] Support parsing `like.object` either as URL or object --- lemmy_apub/src/activities/receive/comment.rs | 40 +++++++++-------- .../src/activities/receive/comment_undo.rs | 31 ++++++------- lemmy_apub/src/activities/receive/mod.rs | 25 +++++++++++ lemmy_apub/src/activities/receive/post.rs | 39 ++++++++-------- .../src/activities/receive/post_undo.rs | 31 ++++++------- .../src/activities/receive/private_message.rs | 4 +- lemmy_apub/src/fetcher.rs | 44 ++++++------------- lemmy_apub/src/objects/comment.rs | 4 +- lemmy_apub/src/objects/community.rs | 4 +- lemmy_apub/src/objects/mod.rs | 26 ++++------- lemmy_apub/src/objects/post.rs | 4 +- lemmy_apub/src/objects/private_message.rs | 4 +- lemmy_apub/src/objects/user.rs | 4 +- 13 files changed, 128 insertions(+), 132 deletions(-) diff --git a/lemmy_apub/src/activities/receive/comment.rs b/lemmy_apub/src/activities/receive/comment.rs index da29a3b7e..1d0260f77 100644 --- a/lemmy_apub/src/activities/receive/comment.rs +++ b/lemmy_apub/src/activities/receive/comment.rs @@ -1,6 +1,20 @@ -use crate::{activities::receive::get_actor_as_user, objects::FromApub, ActorType, NoteExt}; +use crate::{ + activities::receive::{get_actor_as_user, get_like_object_id}, + fetcher::get_or_fetch_and_insert_comment, + objects::FromApub, + ActorType, + NoteExt, +}; use activitystreams::{ - activity::{ActorAndObjectRefExt, Create, Dislike, Like, Remove, Update}, + activity::{ + kind::{DislikeType, LikeType}, + ActorAndObjectRefExt, + Create, + Dislike, + Like, + Remove, + Update, + }, base::ExtendsExt, }; use anyhow::Context; @@ -23,7 +37,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, Some(user.actor_id()?), request_counter).await?; + let comment = Comment::from_apub(¬e, context, 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 +80,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, Some(user.actor_id()?), request_counter).await?; + let comment = Comment::from_apub(¬e, context, user.actor_id()?, request_counter).await?; let comment_id = comment.id; let post_id = comment.post_id; @@ -102,11 +116,9 @@ pub(crate) async fn receive_like_comment( context: &LemmyContext, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let note = NoteExt::from_any_base(like.object().to_owned().one().context(location_info!())?)? - .context(location_info!())?; let user = get_actor_as_user(&like, context, request_counter).await?; - - let comment = Comment::from_apub(¬e, context, None, request_counter).await?; + let comment_id = get_like_object_id::(&like)?; + let comment = get_or_fetch_and_insert_comment(&comment_id, context, request_counter).await?; let comment_id = comment.id; let like_form = CommentLikeForm { @@ -150,17 +162,9 @@ pub(crate) async fn receive_dislike_comment( context: &LemmyContext, request_counter: &mut i32, ) -> Result<(), LemmyError> { - let note = NoteExt::from_any_base( - dislike - .object() - .to_owned() - .one() - .context(location_info!())?, - )? - .context(location_info!())?; let user = get_actor_as_user(&dislike, context, request_counter).await?; - - let comment = Comment::from_apub(¬e, context, None, request_counter).await?; + let comment_id = get_like_object_id::(&dislike)?; + let comment = get_or_fetch_and_insert_comment(&comment_id, context, request_counter).await?; let comment_id = comment.id; let like_form = CommentLikeForm { diff --git a/lemmy_apub/src/activities/receive/comment_undo.rs b/lemmy_apub/src/activities/receive/comment_undo.rs index 2ccec087b..da9246960 100644 --- a/lemmy_apub/src/activities/receive/comment_undo.rs +++ b/lemmy_apub/src/activities/receive/comment_undo.rs @@ -1,13 +1,18 @@ -use crate::{activities::receive::get_actor_as_user, objects::FromApub, NoteExt}; -use activitystreams::{activity::*, prelude::*}; -use anyhow::Context; +use crate::{ + activities::receive::{get_actor_as_user, get_like_object_id}, + fetcher::get_or_fetch_and_insert_comment, +}; +use activitystreams::activity::{ + kind::{DislikeType, LikeType}, + *, +}; use lemmy_db::{ comment::{Comment, CommentLike}, comment_view::CommentView, Likeable, }; use lemmy_structs::{blocking, comment::CommentResponse}; -use lemmy_utils::{location_info, LemmyError}; +use lemmy_utils::LemmyError; use lemmy_websocket::{messages::SendComment, LemmyContext, UserOperation}; pub(crate) async fn receive_undo_like_comment( @@ -16,10 +21,8 @@ pub(crate) async fn receive_undo_like_comment( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(like, context, request_counter).await?; - let note = NoteExt::from_any_base(like.object().to_owned().one().context(location_info!())?)? - .context(location_info!())?; - - let comment = Comment::from_apub(¬e, context, None, request_counter).await?; + let comment_id = get_like_object_id::(like)?; + let comment = get_or_fetch_and_insert_comment(&comment_id, context, request_counter).await?; let comment_id = comment.id; let user_id = user.id; @@ -57,16 +60,8 @@ pub(crate) async fn receive_undo_dislike_comment( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(dislike, context, request_counter).await?; - let note = NoteExt::from_any_base( - dislike - .object() - .to_owned() - .one() - .context(location_info!())?, - )? - .context(location_info!())?; - - let comment = Comment::from_apub(¬e, context, None, request_counter).await?; + let comment_id = get_like_object_id::(dislike)?; + let comment = get_or_fetch_and_insert_comment(&comment_id, context, request_counter).await?; let comment_id = comment.id; let user_id = user.id; diff --git a/lemmy_apub/src/activities/receive/mod.rs b/lemmy_apub/src/activities/receive/mod.rs index 1f17fe9f3..d67f12fac 100644 --- a/lemmy_apub/src/activities/receive/mod.rs +++ b/lemmy_apub/src/activities/receive/mod.rs @@ -79,3 +79,28 @@ where Ok(()) } + +pub(in crate::activities::receive) fn get_like_object_id( + like_or_dislike: &Activity, +) -> Result +where + Activity: ActorAndObjectRefExt, +{ + // For backwards compatibility with older Lemmy versions where like.object contains a full + // post/comment. This can be removed after some time, using + // `activity.oject().as_single_xsd_any_uri()` instead. + let object = like_or_dislike.object(); + if let Some(xsd_uri) = object.as_single_xsd_any_uri() { + Ok(xsd_uri.to_owned()) + } else { + Ok( + object + .to_owned() + .one() + .context(location_info!())? + .id() + .context(location_info!())? + .to_owned(), + ) + } +} diff --git a/lemmy_apub/src/activities/receive/post.rs b/lemmy_apub/src/activities/receive/post.rs index d1c92157f..04d83d374 100644 --- a/lemmy_apub/src/activities/receive/post.rs +++ b/lemmy_apub/src/activities/receive/post.rs @@ -1,6 +1,19 @@ -use crate::{activities::receive::get_actor_as_user, objects::FromApub, ActorType, PageExt}; +use crate::{ + activities::receive::{get_actor_as_user, get_like_object_id}, + fetcher::get_or_fetch_and_insert_post, + objects::FromApub, + ActorType, + PageExt, +}; use activitystreams::{ - activity::{Create, Dislike, Like, Remove, Update}, + activity::{ + kind::{DislikeType, LikeType}, + Create, + Dislike, + Like, + Remove, + Update, + }, prelude::*, }; use anyhow::Context; @@ -22,7 +35,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, Some(user.actor_id()?), request_counter).await?; + let post = Post::from_apub(&page, context, user.actor_id()?, request_counter).await?; // Refetch the view let post_id = post.id; @@ -51,7 +64,7 @@ pub(crate) async fn receive_update_post( let page = PageExt::from_any_base(update.object().to_owned().one().context(location_info!())?)? .context(location_info!())?; - let post = Post::from_apub(&page, context, Some(user.actor_id()?), request_counter).await?; + let post = Post::from_apub(&page, context, user.actor_id()?, request_counter).await?; let post_id = post.id; // Refetch the view @@ -77,10 +90,8 @@ pub(crate) async fn receive_like_post( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(&like, context, request_counter).await?; - let page = PageExt::from_any_base(like.object().to_owned().one().context(location_info!())?)? - .context(location_info!())?; - - let post = Post::from_apub(&page, context, None, request_counter).await?; + let post_id = get_like_object_id::(&like)?; + let post = get_or_fetch_and_insert_post(&post_id, context, request_counter).await?; let post_id = post.id; let like_form = PostLikeForm { @@ -118,16 +129,8 @@ pub(crate) async fn receive_dislike_post( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(&dislike, context, request_counter).await?; - let page = PageExt::from_any_base( - dislike - .object() - .to_owned() - .one() - .context(location_info!())?, - )? - .context(location_info!())?; - - let post = Post::from_apub(&page, context, None, request_counter).await?; + let post_id = get_like_object_id::(&dislike)?; + let post = get_or_fetch_and_insert_post(&post_id, context, request_counter).await?; let post_id = post.id; let like_form = PostLikeForm { diff --git a/lemmy_apub/src/activities/receive/post_undo.rs b/lemmy_apub/src/activities/receive/post_undo.rs index aced5f4b3..cf2f447b1 100644 --- a/lemmy_apub/src/activities/receive/post_undo.rs +++ b/lemmy_apub/src/activities/receive/post_undo.rs @@ -1,13 +1,18 @@ -use crate::{activities::receive::get_actor_as_user, objects::FromApub, PageExt}; -use activitystreams::{activity::*, prelude::*}; -use anyhow::Context; +use crate::{ + activities::receive::{get_actor_as_user, get_like_object_id}, + fetcher::get_or_fetch_and_insert_post, +}; +use activitystreams::activity::{ + kind::{DislikeType, LikeType}, + *, +}; use lemmy_db::{ post::{Post, PostLike}, post_view::PostView, Likeable, }; use lemmy_structs::{blocking, post::PostResponse}; -use lemmy_utils::{location_info, LemmyError}; +use lemmy_utils::LemmyError; use lemmy_websocket::{messages::SendPost, LemmyContext, UserOperation}; pub(crate) async fn receive_undo_like_post( @@ -16,10 +21,8 @@ pub(crate) async fn receive_undo_like_post( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(like, context, request_counter).await?; - let page = PageExt::from_any_base(like.object().to_owned().one().context(location_info!())?)? - .context(location_info!())?; - - let post = Post::from_apub(&page, context, None, request_counter).await?; + let post_id = get_like_object_id::(&like)?; + let post = get_or_fetch_and_insert_post(&post_id, context, request_counter).await?; let post_id = post.id; let user_id = user.id; @@ -51,16 +54,8 @@ pub(crate) async fn receive_undo_dislike_post( request_counter: &mut i32, ) -> Result<(), LemmyError> { let user = get_actor_as_user(dislike, context, request_counter).await?; - let page = PageExt::from_any_base( - dislike - .object() - .to_owned() - .one() - .context(location_info!())?, - )? - .context(location_info!())?; - - let post = Post::from_apub(&page, context, None, request_counter).await?; + let post_id = get_like_object_id::(&dislike)?; + let post = get_or_fetch_and_insert_post(&post_id, context, request_counter).await?; let post_id = post.id; let user_id = user.id; diff --git a/lemmy_apub/src/activities/receive/private_message.rs b/lemmy_apub/src/activities/receive/private_message.rs index 74ce004ae..25ccc520d 100644 --- a/lemmy_apub/src/activities/receive/private_message.rs +++ b/lemmy_apub/src/activities/receive/private_message.rs @@ -37,7 +37,7 @@ pub(crate) async fn receive_create_private_message( .context(location_info!())?; let private_message = - PrivateMessage::from_apub(¬e, context, Some(expected_domain), request_counter).await?; + PrivateMessage::from_apub(¬e, context, expected_domain, request_counter).await?; let message = blocking(&context.pool(), move |conn| { PrivateMessageView::read(conn, private_message.id) @@ -74,7 +74,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, Some(expected_domain), request_counter).await?; + PrivateMessage::from_apub(¬e, context, expected_domain, request_counter).await?; let private_message_id = private_message.id; let message = blocking(&context.pool(), move |conn| { diff --git a/lemmy_apub/src/fetcher.rs b/lemmy_apub/src/fetcher.rs index a29c1fe97..ad977f5b1 100644 --- a/lemmy_apub/src/fetcher.rs +++ b/lemmy_apub/src/fetcher.rs @@ -184,7 +184,7 @@ pub async fn search_by_apub_id( response } SearchAcceptedObjects::Page(p) => { - let p = Post::from_apub(&p, context, Some(query_url), recursion_counter).await?; + let p = Post::from_apub(&p, context, query_url, recursion_counter).await?; response.posts = vec![blocking(context.pool(), move |conn| PostView::read(conn, p.id, None)).await??]; @@ -192,7 +192,7 @@ pub async fn search_by_apub_id( response } SearchAcceptedObjects::Comment(c) => { - let c = Comment::from_apub(&c, context, Some(query_url), recursion_counter).await?; + let c = Comment::from_apub(&c, context, query_url, recursion_counter).await?; response.comments = vec![ blocking(context.pool(), move |conn| { @@ -252,13 +252,7 @@ pub(crate) async fn get_or_fetch_and_upsert_user( return Ok(u); } - let user = User_::from_apub( - &person?, - context, - Some(apub_id.to_owned()), - recursion_counter, - ) - .await?; + let user = User_::from_apub(&person?, context, apub_id.to_owned(), recursion_counter).await?; let user_id = user.id; blocking(context.pool(), move |conn| { @@ -274,13 +268,7 @@ 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, - Some(apub_id.to_owned()), - recursion_counter, - ) - .await?; + let user = User_::from_apub(&person, context, apub_id.to_owned(), recursion_counter).await?; Ok(user) } @@ -352,7 +340,7 @@ async fn fetch_remote_community( let group = group?; let community = - Community::from_apub(&group, context, Some(apub_id.to_owned()), recursion_counter).await?; + Community::from_apub(&group, context, apub_id.to_owned(), recursion_counter).await?; // Also add the community moderators too let attributed_to = group.inner.attributed_to().context(location_info!())?; @@ -402,13 +390,13 @@ async fn fetch_remote_community( } for o in outbox_items { let page = PageExt::from_any_base(o)?.context(location_info!())?; + let page_id = page.id_unchecked().context(location_info!())?; - // The post creator may be from a blocked instance, - // if it errors, then continue - match Post::from_apub(&page, context, None, recursion_counter).await { - Ok(post) => post, - Err(_) => continue, - }; + // The post creator may be from a blocked instance, if it errors, then skip it + if check_is_apub_id_valid(page_id).is_err() { + continue; + } + Post::from_apub(&page, context, page_id.to_owned(), recursion_counter).await?; // TODO: we need to send a websocket update here } @@ -436,13 +424,7 @@ 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, - Some(post_ap_id.to_owned()), - recursion_counter, - ) - .await?; + let post = Post::from_apub(&page, context, post_ap_id.to_owned(), recursion_counter).await?; Ok(post) } @@ -477,7 +459,7 @@ pub(crate) async fn get_or_fetch_and_insert_comment( let comment = Comment::from_apub( &comment, context, - Some(comment_ap_id.to_owned()), + comment_ap_id.to_owned(), recursion_counter, ) .await?; diff --git a/lemmy_apub/src/objects/comment.rs b/lemmy_apub/src/objects/comment.rs index 9ff63cbd2..56d75a404 100644 --- a/lemmy_apub/src/objects/comment.rs +++ b/lemmy_apub/src/objects/comment.rs @@ -99,7 +99,7 @@ impl FromApub for Comment { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { check_object_for_community_or_site_ban(note, context, request_counter).await?; @@ -128,7 +128,7 @@ impl FromApubToForm for CommentForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { let creator_actor_id = ¬e diff --git a/lemmy_apub/src/objects/community.rs b/lemmy_apub/src/objects/community.rs index fe05de9e4..594a5b5ee 100644 --- a/lemmy_apub/src/objects/community.rs +++ b/lemmy_apub/src/objects/community.rs @@ -117,7 +117,7 @@ impl FromApub for Community { async fn from_apub( group: &GroupExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { get_object_from_apub(group, context, expected_domain, request_counter).await @@ -129,7 +129,7 @@ impl FromApubToForm for CommunityForm { async fn from_apub( group: &GroupExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { let creator_and_moderator_uris = group.inner.attributed_to().context(location_info!())?; diff --git a/lemmy_apub/src/objects/mod.rs b/lemmy_apub/src/objects/mod.rs index fbe89b09a..898c50f31 100644 --- a/lemmy_apub/src/objects/mod.rs +++ b/lemmy_apub/src/objects/mod.rs @@ -38,12 +38,11 @@ pub(crate) trait FromApub { /// /// * `apub` The object to read from /// * `context` LemmyContext which holds DB pool, HTTP client etc - /// * `expected_domain` If present, ensure that the domains of this and of the apub object ID are - /// identical + /// * `expected_domain` Domain where the object was received from async fn from_apub( apub: &Self::ApubType, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result where @@ -55,7 +54,7 @@ pub(in crate::objects) trait FromApubToForm { async fn from_apub( apub: &ApubType, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result where @@ -89,17 +88,13 @@ where pub(in crate::objects) fn check_object_domain( apub: &T, - expected_domain: Option, + expected_domain: Url, ) -> Result where T: Base + AsBase, { - let object_id = if let Some(url) = expected_domain { - let domain = url.domain().context(location_info!())?; - apub.id(domain)?.context(location_info!())? - } else { - apub.id_unchecked().context(location_info!())? - }; + let domain = expected_domain.domain().context(location_info!())?; + let object_id = apub.id(domain)?.context(location_info!())?; check_is_apub_id_valid(&object_id)?; Ok(object_id.to_string()) } @@ -180,7 +175,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: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result where @@ -191,7 +186,7 @@ where let object_id = from.id_unchecked().context(location_info!())?.to_owned(); let domain = object_id.domain().context(location_info!())?; - // if we already have the object in our database, return that directly + // if its a local object, return it directly from the database if Settings::get().hostname == domain { let object = blocking(context.pool(), move |conn| { To::read_from_apub_id(conn, object_id.as_str()) @@ -199,10 +194,7 @@ where .await??; Ok(object) } - // if we dont have it, parse from apub and insert into database - // TODO: this is insecure, a `Like/Post` could result in a non-existent post from a different - // instance being inserted into our database. we should request the object over http in that - // case. this might happen exactly in the case where expected_domain = None, but i'm not sure. + // otherwise parse and insert, assuring that it comes from the right domain else { let to_form = ToForm::from_apub(&from, context, expected_domain, request_counter).await?; diff --git a/lemmy_apub/src/objects/post.rs b/lemmy_apub/src/objects/post.rs index 3ca8650da..d34098c5b 100644 --- a/lemmy_apub/src/objects/post.rs +++ b/lemmy_apub/src/objects/post.rs @@ -110,7 +110,7 @@ impl FromApub for Post { async fn from_apub( page: &PageExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { check_object_for_community_or_site_ban(page, context, request_counter).await?; @@ -123,7 +123,7 @@ impl FromApubToForm for PostForm { async fn from_apub( page: &PageExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { let ext = &page.ext_one; diff --git a/lemmy_apub/src/objects/private_message.rs b/lemmy_apub/src/objects/private_message.rs index 84b5f9803..ec8b55e7e 100644 --- a/lemmy_apub/src/objects/private_message.rs +++ b/lemmy_apub/src/objects/private_message.rs @@ -71,7 +71,7 @@ impl FromApub for PrivateMessage { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { get_object_from_apub(note, context, expected_domain, request_counter).await @@ -83,7 +83,7 @@ impl FromApubToForm for PrivateMessageForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { let creator_actor_id = note diff --git a/lemmy_apub/src/objects/user.rs b/lemmy_apub/src/objects/user.rs index f65df242a..18490796a 100644 --- a/lemmy_apub/src/objects/user.rs +++ b/lemmy_apub/src/objects/user.rs @@ -95,7 +95,7 @@ impl FromApub for User_ { async fn from_apub( person: &PersonExt, context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, request_counter: &mut i32, ) -> Result { let user_id = person.id_unchecked().context(location_info!())?.to_owned(); @@ -120,7 +120,7 @@ impl FromApubToForm for UserForm { async fn from_apub( person: &PersonExt, _context: &LemmyContext, - expected_domain: Option, + expected_domain: Url, _request_counter: &mut i32, ) -> Result { let avatar = match person.icon() {