From 969a7f2d1b3cdc85e1f8438ba2261212b5774a6d Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 3 Nov 2021 17:26:09 +0100 Subject: [PATCH] Refactoring apub code --- .../activities/comment/create_or_update.rs | 2 +- .../src/activities/post/create_or_update.rs | 4 ++-- .../private_message/create_or_update.rs | 2 +- crates/apub/src/objects/comment.rs | 8 +++++--- crates/apub/src/objects/community.rs | 2 +- crates/apub/src/objects/person.rs | 7 ++++--- crates/apub/src/objects/post.rs | 13 ++++++------- crates/apub/src/objects/private_message.rs | 6 ++++-- .../apub/src/protocol/objects/chat_message.rs | 19 +++++++------------ crates/apub/src/protocol/objects/group.rs | 8 ++++---- crates/apub/src/protocol/objects/note.rs | 17 +++-------------- crates/apub/src/protocol/objects/page.rs | 12 ++---------- crates/apub/src/protocol/objects/person.rs | 8 ++++++-- 13 files changed, 46 insertions(+), 62 deletions(-) diff --git a/crates/apub/src/activities/comment/create_or_update.rs b/crates/apub/src/activities/comment/create_or_update.rs index a9c1a9ee8..10666932e 100644 --- a/crates/apub/src/activities/comment/create_or_update.rs +++ b/crates/apub/src/activities/comment/create_or_update.rs @@ -83,7 +83,7 @@ impl ActivityHandler for CreateOrUpdateComment { verify_activity(self, &context.settings())?; verify_person_in_community(&self.actor, &community, context, request_counter).await?; - verify_domains_match(self.actor.inner(), self.object.id_unchecked())?; + verify_domains_match(self.actor.inner(), self.object.id.inner())?; check_community_deleted_or_removed(&community)?; check_post_deleted_or_removed(&post)?; diff --git a/crates/apub/src/activities/post/create_or_update.rs b/crates/apub/src/activities/post/create_or_update.rs index 41590493c..677ce2dc1 100644 --- a/crates/apub/src/activities/post/create_or_update.rs +++ b/crates/apub/src/activities/post/create_or_update.rs @@ -84,7 +84,7 @@ impl ActivityHandler for CreateOrUpdatePost { match self.kind { CreateOrUpdateType::Create => { - verify_domains_match(self.actor.inner(), self.object.id_unchecked())?; + verify_domains_match(self.actor.inner(), self.object.id.inner())?; verify_urls_match(self.actor(), self.object.attributed_to.inner())?; // Check that the post isnt locked or stickied, as that isnt possible for newly created posts. // However, when fetching a remote post we generate a new create activity with the current @@ -101,7 +101,7 @@ impl ActivityHandler for CreateOrUpdatePost { if is_mod_action { verify_mod_action(&self.actor, &community, context, request_counter).await?; } else { - verify_domains_match(self.actor.inner(), self.object.id_unchecked())?; + verify_domains_match(self.actor.inner(), self.object.id.inner())?; verify_urls_match(self.actor(), self.object.attributed_to.inner())?; } } diff --git a/crates/apub/src/activities/private_message/create_or_update.rs b/crates/apub/src/activities/private_message/create_or_update.rs index cfd7c8bcf..e441718ab 100644 --- a/crates/apub/src/activities/private_message/create_or_update.rs +++ b/crates/apub/src/activities/private_message/create_or_update.rs @@ -56,7 +56,7 @@ impl ActivityHandler for CreateOrUpdatePrivateMessage { ) -> Result<(), LemmyError> { verify_activity(self, &context.settings())?; verify_person(&self.actor, context, request_counter).await?; - verify_domains_match(self.actor.inner(), self.object.id_unchecked())?; + verify_domains_match(self.actor.inner(), self.object.id.inner())?; self.object.verify(context, request_counter).await?; Ok(()) } diff --git a/crates/apub/src/objects/comment.rs b/crates/apub/src/objects/comment.rs index 08a55bf9c..68240728a 100644 --- a/crates/apub/src/objects/comment.rs +++ b/crates/apub/src/objects/comment.rs @@ -39,6 +39,7 @@ use crate::{ }, PostOrComment, }; +use lemmy_apub_lib::verify::verify_domains_match; use lemmy_utils::utils::markdown_to_html; #[derive(Clone, Debug)] @@ -107,7 +108,7 @@ impl ApubObject for ApubComment { let note = Note { r#type: NoteType::Note, - id: self.ap_id.to_owned().into_inner(), + id: ObjectId::new(self.ap_id.to_owned()), attributed_to: ObjectId::new(creator.actor_id), to: vec![public()], content: markdown_to_html(&self.content), @@ -141,7 +142,8 @@ impl ApubObject for ApubComment { expected_domain: &Url, request_counter: &mut i32, ) -> Result { - let ap_id = Some(note.id(expected_domain)?.clone().into()); + verify_domains_match(note.id.inner(), expected_domain)?; + let ap_id = Some(note.id.clone().into()); let creator = note .attributed_to .dereference(context, request_counter) @@ -152,7 +154,7 @@ impl ApubObject for ApubComment { Community::read(conn, community_id) }) .await??; - check_is_apub_id_valid(¬e.id, community.local, &context.settings())?; + check_is_apub_id_valid(note.id.inner(), community.local, &context.settings())?; verify_person_in_community( ¬e.attributed_to, &community.into(), diff --git a/crates/apub/src/objects/community.rs b/crates/apub/src/objects/community.rs index 0947ebf37..8fd245dc2 100644 --- a/crates/apub/src/objects/community.rs +++ b/crates/apub/src/objects/community.rs @@ -96,7 +96,7 @@ impl ApubObject for ApubCommunity { let group = Group { kind: GroupType::Group, - id: self.actor_id(), + id: ObjectId::new(self.actor_id()), preferred_username: self.name.clone(), name: self.title.clone(), summary: self.description.as_ref().map(|b| markdown_to_html(b)), diff --git a/crates/apub/src/objects/person.rs b/crates/apub/src/objects/person.rs index 18522e569..652ef6220 100644 --- a/crates/apub/src/objects/person.rs +++ b/crates/apub/src/objects/person.rs @@ -1,5 +1,6 @@ use crate::{ check_is_apub_id_valid, + fetcher::object_id::ObjectId, generate_outbox_url, objects::get_summary_from_string_or_source, protocol::{ @@ -96,7 +97,7 @@ impl ApubObject for ApubPerson { let person = Person { kind, - id: self.actor_id.to_owned().into_inner(), + id: ObjectId::new(self.actor_id.clone()), preferred_username: self.name.clone(), name: self.display_name.clone(), summary: self.bio.as_ref().map(|b| markdown_to_html(b)), @@ -128,7 +129,7 @@ impl ApubObject for ApubPerson { expected_domain: &Url, _request_counter: &mut i32, ) -> Result { - verify_domains_match(&person.id, expected_domain)?; + verify_domains_match(person.id.inner(), expected_domain)?; let actor_id = Some(person.id.clone().into()); let name = person.preferred_username.clone(); let display_name: Option = person.name.clone(); @@ -144,7 +145,7 @@ impl ApubObject for ApubPerson { check_slurs_opt(&display_name, slur_regex)?; check_slurs_opt(&bio, slur_regex)?; - check_is_apub_id_valid(&person.id, false, &context.settings())?; + check_is_apub_id_valid(person.id.inner(), false, &context.settings())?; let person_form = PersonForm { name, diff --git a/crates/apub/src/objects/post.rs b/crates/apub/src/objects/post.rs index b835f812e..b29bdb801 100644 --- a/crates/apub/src/objects/post.rs +++ b/crates/apub/src/objects/post.rs @@ -17,6 +17,7 @@ use lemmy_api_common::blocking; use lemmy_apub_lib::{ traits::ApubObject, values::{MediaTypeHtml, MediaTypeMarkdown}, + verify::verify_domains_match, }; use lemmy_db_schema::{ self, @@ -106,7 +107,7 @@ impl ApubObject for ApubPost { let page = Page { r#type: PageType::Page, - id: self.ap_id.clone().into(), + id: ObjectId::new(self.ap_id.clone()), attributed_to: ObjectId::new(creator.actor_id), to: vec![community.actor_id.into(), public()], name: self.name.clone(), @@ -140,18 +141,16 @@ impl ApubObject for ApubPost { ) -> Result { // We can't verify the domain in case of mod action, because the mod may be on a different // instance from the post author. - let ap_id = if page.is_mod_action(context).await? { - page.id_unchecked() - } else { - page.id(expected_domain)? + if !page.is_mod_action(context).await? { + verify_domains_match(page.id.inner(), expected_domain)?; }; - let ap_id = Some(ap_id.clone().into()); + let ap_id = Some(page.id.clone().into()); let creator = page .attributed_to .dereference(context, request_counter) .await?; let community = page.extract_community(context, request_counter).await?; - check_is_apub_id_valid(&page.id, community.local, &context.settings())?; + check_is_apub_id_valid(page.id.inner(), community.local, &context.settings())?; verify_person_in_community(&page.attributed_to, &community, context, request_counter).await?; let thumbnail_url: Option = page.image.clone().map(|i| i.url); diff --git a/crates/apub/src/objects/private_message.rs b/crates/apub/src/objects/private_message.rs index 81615fdc9..5c4befb05 100644 --- a/crates/apub/src/objects/private_message.rs +++ b/crates/apub/src/objects/private_message.rs @@ -11,6 +11,7 @@ use lemmy_api_common::blocking; use lemmy_apub_lib::{ traits::ApubObject, values::{MediaTypeHtml, MediaTypeMarkdown}, + verify::verify_domains_match, }; use lemmy_db_schema::{ source::{ @@ -81,7 +82,7 @@ impl ApubObject for ApubPrivateMessage { let note = ChatMessage { r#type: ChatMessageType::ChatMessage, - id: self.ap_id.clone().into(), + id: ObjectId::new(self.ap_id.clone()), attributed_to: ObjectId::new(creator.actor_id), to: [ObjectId::new(recipient.actor_id)], content: markdown_to_html(&self.content), @@ -107,7 +108,8 @@ impl ApubObject for ApubPrivateMessage { expected_domain: &Url, request_counter: &mut i32, ) -> Result { - let ap_id = Some(note.id(expected_domain)?.clone().into()); + verify_domains_match(note.id.inner(), expected_domain)?; + let ap_id = Some(note.id.clone().into()); let creator = note .attributed_to .dereference(context, request_counter) diff --git a/crates/apub/src/protocol/objects/chat_message.rs b/crates/apub/src/protocol/objects/chat_message.rs index 038af4edf..e9677758c 100644 --- a/crates/apub/src/protocol/objects/chat_message.rs +++ b/crates/apub/src/protocol/objects/chat_message.rs @@ -1,4 +1,8 @@ -use crate::{fetcher::object_id::ObjectId, objects::person::ApubPerson, protocol::Source}; +use crate::{ + fetcher::object_id::ObjectId, + objects::{person::ApubPerson, private_message::ApubPrivateMessage}, + protocol::Source, +}; use activitystreams::{ chrono::{DateTime, FixedOffset}, unparsed::Unparsed, @@ -9,14 +13,13 @@ use lemmy_utils::LemmyError; use lemmy_websocket::LemmyContext; use serde::{Deserialize, Serialize}; use serde_with::skip_serializing_none; -use url::Url; #[skip_serializing_none] #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(rename_all = "camelCase")] pub struct ChatMessage { pub(crate) r#type: ChatMessageType, - pub(crate) id: Url, + pub(crate) id: ObjectId, pub(crate) attributed_to: ObjectId, pub(crate) to: [ObjectId; 1], pub(crate) content: String, @@ -35,20 +38,12 @@ pub enum ChatMessageType { } impl ChatMessage { - pub(crate) fn id_unchecked(&self) -> &Url { - &self.id - } - pub(crate) fn id(&self, expected_domain: &Url) -> Result<&Url, LemmyError> { - verify_domains_match(&self.id, expected_domain)?; - Ok(&self.id) - } - pub(crate) async fn verify( &self, context: &LemmyContext, request_counter: &mut i32, ) -> Result<(), LemmyError> { - verify_domains_match(self.attributed_to.inner(), &self.id)?; + verify_domains_match(self.attributed_to.inner(), self.id.inner())?; let person = self .attributed_to .dereference(context, request_counter) diff --git a/crates/apub/src/protocol/objects/group.rs b/crates/apub/src/protocol/objects/group.rs index 4da987a25..921a5dc8d 100644 --- a/crates/apub/src/protocol/objects/group.rs +++ b/crates/apub/src/protocol/objects/group.rs @@ -5,7 +5,7 @@ use crate::{ community_outbox::ApubCommunityOutbox, }, fetcher::object_id::ObjectId, - objects::get_summary_from_string_or_source, + objects::{community::ApubCommunity, get_summary_from_string_or_source}, protocol::{ImageObject, Source}, }; use activitystreams::{ @@ -30,7 +30,7 @@ use url::Url; pub struct Group { #[serde(rename = "type")] pub(crate) kind: GroupType, - pub(crate) id: Url, + pub(crate) id: ObjectId, /// username, set at account creation and can never be changed pub(crate) preferred_username: String, /// title (can be changed at any time) @@ -61,8 +61,8 @@ impl Group { expected_domain: &Url, settings: &Settings, ) -> Result { - check_is_apub_id_valid(&group.id, true, settings)?; - verify_domains_match(expected_domain, &group.id)?; + check_is_apub_id_valid(group.id.inner(), true, settings)?; + verify_domains_match(expected_domain, group.id.inner())?; let name = group.preferred_username.clone(); let title = group.name.clone(); let description = get_summary_from_string_or_source(&group.summary, &group.source); diff --git a/crates/apub/src/protocol/objects/note.rs b/crates/apub/src/protocol/objects/note.rs index bdc4da66b..58b164a7d 100644 --- a/crates/apub/src/protocol/objects/note.rs +++ b/crates/apub/src/protocol/objects/note.rs @@ -1,7 +1,7 @@ use crate::{ activities::{verify_is_public, verify_person_in_community}, fetcher::{object_id::ObjectId, post_or_comment::PostOrComment}, - objects::{community::ApubCommunity, person::ApubPerson, post::ApubPost}, + objects::{comment::ApubComment, community::ApubCommunity, person::ApubPerson, post::ApubPost}, protocol::Source, }; use activitystreams::{object::kind::NoteType, unparsed::Unparsed}; @@ -26,11 +26,8 @@ use url::Url; #[serde(rename_all = "camelCase")] pub struct Note { pub(crate) r#type: NoteType, - pub(crate) id: Url, + pub(crate) id: ObjectId, pub(crate) attributed_to: ObjectId, - /// Indicates that the object is publicly readable. Unlike [`Post.to`], this one doesn't contain - /// the community ID, as it would be incompatible with Pleroma (and we can get the community from - /// the post in [`in_reply_to`]). pub(crate) to: Vec, pub(crate) content: String, pub(crate) media_type: Option, @@ -52,14 +49,6 @@ pub(crate) enum SourceCompat { } impl Note { - pub(crate) fn id_unchecked(&self) -> &Url { - &self.id - } - pub(crate) fn id(&self, expected_domain: &Url) -> Result<&Url, LemmyError> { - verify_domains_match(&self.id, expected_domain)?; - Ok(&self.id) - } - pub(crate) async fn get_parents( &self, context: &LemmyContext, @@ -104,7 +93,7 @@ impl Note { if post.locked { return Err(anyhow!("Post is locked").into()); } - verify_domains_match(self.attributed_to.inner(), &self.id)?; + verify_domains_match(self.attributed_to.inner(), self.id.inner())?; verify_person_in_community(&self.attributed_to, &community, context, request_counter).await?; verify_is_public(&self.to)?; Ok(()) diff --git a/crates/apub/src/protocol/objects/page.rs b/crates/apub/src/protocol/objects/page.rs index 7887f19c1..0cd004799 100644 --- a/crates/apub/src/protocol/objects/page.rs +++ b/crates/apub/src/protocol/objects/page.rs @@ -19,7 +19,7 @@ use url::Url; #[serde(rename_all = "camelCase")] pub struct Page { pub(crate) r#type: PageType, - pub(crate) id: Url, + pub(crate) id: ObjectId, pub(crate) attributed_to: ObjectId, pub(crate) to: Vec, pub(crate) name: String, @@ -38,14 +38,6 @@ pub struct Page { } impl Page { - pub(crate) fn id_unchecked(&self) -> &Url { - &self.id - } - pub(crate) fn id(&self, expected_domain: &Url) -> Result<&Url, LemmyError> { - verify_domains_match(&self.id, expected_domain)?; - Ok(&self.id) - } - /// Only mods can change the post's stickied/locked status. So if either of these is changed from /// the current value, it is a mod action and needs to be verified as such. /// @@ -71,7 +63,7 @@ impl Page { let community = self.extract_community(context, request_counter).await?; check_slurs(&self.name, &context.settings().slur_regex())?; - verify_domains_match(self.attributed_to.inner(), &self.id.clone())?; + verify_domains_match(self.attributed_to.inner(), self.id.inner())?; verify_person_in_community(&self.attributed_to, &community, context, request_counter).await?; verify_is_public(&self.to.clone())?; Ok(()) diff --git a/crates/apub/src/protocol/objects/person.rs b/crates/apub/src/protocol/objects/person.rs index 2aecf945e..720e7de00 100644 --- a/crates/apub/src/protocol/objects/person.rs +++ b/crates/apub/src/protocol/objects/person.rs @@ -1,4 +1,8 @@ -use crate::protocol::{ImageObject, Source}; +use crate::{ + fetcher::object_id::ObjectId, + objects::person::ApubPerson, + protocol::{ImageObject, Source}, +}; use activitystreams::{actor::Endpoints, unparsed::Unparsed, url::Url}; use chrono::{DateTime, FixedOffset}; use lemmy_apub_lib::signatures::PublicKey; @@ -17,7 +21,7 @@ pub enum UserTypes { pub struct Person { #[serde(rename = "type")] pub(crate) kind: UserTypes, - pub(crate) id: Url, + pub(crate) id: ObjectId, /// username, set at account creation and can never be changed pub(crate) preferred_username: String, /// displayname (can be changed at any time)