From ddf480d6e25e58706bbc7845df3df99642f0964e Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Sat, 10 Jul 2021 01:33:18 +0200 Subject: [PATCH] rework following verify(), fix tests and test manually --- .../src/activities/comment/like.rs | 2 +- .../src/activities/following/accept.rs | 19 +++++----- .../src/activities/following/follow.rs | 21 ++++++----- .../src/activities/following/undo.rs | 20 ++++++----- crates/apub_receive/src/activities/mod.rs | 35 +++++++++++++++++++ .../src/activities/private_message/create.rs | 2 +- .../src/activities/private_message/delete.rs | 2 +- .../src/activities/private_message/mod.rs | 24 ------------- .../activities/private_message/undo_delete.rs | 5 ++- .../src/activities/private_message/update.rs | 2 +- 10 files changed, 76 insertions(+), 56 deletions(-) diff --git a/crates/apub_receive/src/activities/comment/like.rs b/crates/apub_receive/src/activities/comment/like.rs index ce45f10ff..e8c9956dc 100644 --- a/crates/apub_receive/src/activities/comment/like.rs +++ b/crates/apub_receive/src/activities/comment/like.rs @@ -31,7 +31,7 @@ impl ActivityHandlerNew for LikeComment { request_counter: &mut i32, ) -> Result<(), LemmyError> { like_or_dislike_comment( - -1, + 1, &self.common.actor, &self.object, context, diff --git a/crates/apub_receive/src/activities/following/accept.rs b/crates/apub_receive/src/activities/following/accept.rs index 74b81777d..bd0aa9cb8 100644 --- a/crates/apub_receive/src/activities/following/accept.rs +++ b/crates/apub_receive/src/activities/following/accept.rs @@ -1,11 +1,11 @@ -use crate::activities::following::follow::FollowCommunity; +use crate::activities::{following::follow::FollowCommunity, verify_activity, verify_community}; use activitystreams::activity::kind::AcceptType; use lemmy_api_common::blocking; -use lemmy_apub::{ - check_is_apub_id_valid, - fetcher::{community::get_or_fetch_and_upsert_community, person::get_or_fetch_and_upsert_person}, +use lemmy_apub::fetcher::{ + community::get_or_fetch_and_upsert_community, + person::get_or_fetch_and_upsert_person, }; -use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew}; +use lemmy_apub_lib::{verify_urls_match, ActivityCommonFields, ActivityHandlerNew}; use lemmy_db_queries::Followable; use lemmy_db_schema::source::community::CommunityFollower; use lemmy_utils::LemmyError; @@ -31,9 +31,12 @@ impl ActivityHandlerNew for AcceptFollowCommunity { context: &LemmyContext, request_counter: &mut i32, ) -> Result<(), LemmyError> { - verify_domains_match(&self.common.actor, self.common.id_unchecked())?; - check_is_apub_id_valid(&self.common.actor, false)?; - self.object.verify(context, request_counter).await + verify_activity(self.common())?; + verify_urls_match(&self.to, &self.object.common.actor)?; + verify_urls_match(&self.common.actor, &self.object.to)?; + verify_community(&self.common.actor, context, request_counter).await?; + self.object.verify(context, request_counter).await?; + Ok(()) } async fn receive( diff --git a/crates/apub_receive/src/activities/following/follow.rs b/crates/apub_receive/src/activities/following/follow.rs index 598d8d20e..f9ca8d429 100644 --- a/crates/apub_receive/src/activities/following/follow.rs +++ b/crates/apub_receive/src/activities/following/follow.rs @@ -1,3 +1,4 @@ +use crate::activities::{verify_activity, verify_person}; use activitystreams::{ activity::{kind::FollowType, Follow}, base::{AnyBase, ExtendsExt}, @@ -5,11 +6,10 @@ use activitystreams::{ use anyhow::Context; use lemmy_api_common::blocking; use lemmy_apub::{ - check_is_apub_id_valid, fetcher::{community::get_or_fetch_and_upsert_community, person::get_or_fetch_and_upsert_person}, CommunityType, }; -use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew}; +use lemmy_apub_lib::{verify_urls_match, ActivityCommonFields, ActivityHandlerNew}; use lemmy_db_queries::Followable; use lemmy_db_schema::source::community::{CommunityFollower, CommunityFollowerForm}; use lemmy_utils::{location_info, LemmyError}; @@ -19,20 +19,25 @@ use url::Url; #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] #[serde(rename_all = "camelCase")] pub struct FollowCommunity { - to: Url, + pub(in crate::activities::following) to: Url, pub(in crate::activities::following) object: Url, #[serde(rename = "type")] kind: FollowType, #[serde(flatten)] - common: ActivityCommonFields, + pub(in crate::activities::following) common: ActivityCommonFields, } #[async_trait::async_trait(?Send)] impl ActivityHandlerNew for FollowCommunity { - async fn verify(&self, _context: &LemmyContext, _: &mut i32) -> Result<(), LemmyError> { - verify_domains_match(&self.common.actor, self.common.id_unchecked())?; - verify_domains_match(&self.to, &self.object)?; - check_is_apub_id_valid(&self.common.actor, false) + async fn verify( + &self, + context: &LemmyContext, + request_counter: &mut i32, + ) -> Result<(), LemmyError> { + verify_activity(self.common())?; + verify_urls_match(&self.to, &self.object)?; + verify_person(&self.common.actor, context, request_counter).await?; + Ok(()) } async fn receive( diff --git a/crates/apub_receive/src/activities/following/undo.rs b/crates/apub_receive/src/activities/following/undo.rs index 9923fd58f..1e25669e3 100644 --- a/crates/apub_receive/src/activities/following/undo.rs +++ b/crates/apub_receive/src/activities/following/undo.rs @@ -1,11 +1,11 @@ -use crate::activities::following::follow::FollowCommunity; +use crate::activities::{following::follow::FollowCommunity, verify_activity, verify_person}; use activitystreams::activity::kind::UndoType; use lemmy_api_common::blocking; -use lemmy_apub::{ - check_is_apub_id_valid, - fetcher::{community::get_or_fetch_and_upsert_community, person::get_or_fetch_and_upsert_person}, +use lemmy_apub::fetcher::{ + community::get_or_fetch_and_upsert_community, + person::get_or_fetch_and_upsert_person, }; -use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew}; +use lemmy_apub_lib::{verify_urls_match, ActivityCommonFields, ActivityHandlerNew}; use lemmy_db_queries::Followable; use lemmy_db_schema::source::community::{CommunityFollower, CommunityFollowerForm}; use lemmy_utils::LemmyError; @@ -30,10 +30,12 @@ impl ActivityHandlerNew for UndoFollowCommunity { context: &LemmyContext, request_counter: &mut i32, ) -> Result<(), LemmyError> { - verify_domains_match(&self.common.actor, self.common.id_unchecked())?; - verify_domains_match(&self.to, &self.object.object)?; - check_is_apub_id_valid(&self.common.actor, false)?; - self.object.verify(context, request_counter).await + verify_activity(self.common())?; + verify_urls_match(&self.to, &self.object.object)?; + verify_urls_match(&self.common.actor, &self.object.common.actor)?; + verify_person(&self.common.actor, context, request_counter).await?; + self.object.verify(context, request_counter).await?; + Ok(()) } async fn receive( diff --git a/crates/apub_receive/src/activities/mod.rs b/crates/apub_receive/src/activities/mod.rs index ed137f79f..eccb5a914 100644 --- a/crates/apub_receive/src/activities/mod.rs +++ b/crates/apub_receive/src/activities/mod.rs @@ -1,5 +1,10 @@ use anyhow::anyhow; use lemmy_api_common::blocking; +use lemmy_apub::{ + check_is_apub_id_valid, + fetcher::{community::get_or_fetch_and_upsert_community, person::get_or_fetch_and_upsert_person}, +}; +use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields}; use lemmy_db_queries::ApubObject; use lemmy_db_schema::source::{community::Community, person::Person}; use lemmy_db_views_actor::community_view::CommunityView; @@ -13,6 +18,36 @@ pub mod following; pub mod post; pub mod private_message; +/// Checks that the specified Url actually identifies a Person (by fetching it), and that the person +/// doesn't have a site ban. +async fn verify_person( + person_id: &Url, + context: &LemmyContext, + request_counter: &mut i32, +) -> Result<(), LemmyError> { + let person = get_or_fetch_and_upsert_person(person_id, context, request_counter).await?; + if person.banned { + return Err(anyhow!("Person {} is banned", person_id).into()); + } + Ok(()) +} + +/// Simply check that the url actually refers to a valid group. +async fn verify_community( + community_id: &Url, + context: &LemmyContext, + request_counter: &mut i32, +) -> Result<(), LemmyError> { + get_or_fetch_and_upsert_community(community_id, context, request_counter).await?; + Ok(()) +} + +fn verify_activity(common: &ActivityCommonFields) -> Result<(), LemmyError> { + check_is_apub_id_valid(&common.actor, false)?; + verify_domains_match(common.id_unchecked(), &common.actor)?; + Ok(()) +} + async fn verify_mod_action( actor_id: Url, activity_cc: Url, diff --git a/crates/apub_receive/src/activities/private_message/create.rs b/crates/apub_receive/src/activities/private_message/create.rs index 2cf732155..2829a1cd5 100644 --- a/crates/apub_receive/src/activities/private_message/create.rs +++ b/crates/apub_receive/src/activities/private_message/create.rs @@ -1,4 +1,4 @@ -use crate::activities::private_message::{send_websocket_message, verify_activity, verify_person}; +use crate::activities::{private_message::send_websocket_message, verify_activity, verify_person}; use activitystreams::{activity::kind::CreateType, base::BaseExt}; use lemmy_apub::{objects::FromApub, NoteExt}; use lemmy_apub_lib::{verify_domains_match_opt, ActivityCommonFields, ActivityHandlerNew}; diff --git a/crates/apub_receive/src/activities/private_message/delete.rs b/crates/apub_receive/src/activities/private_message/delete.rs index db16a7a6a..75fc146c4 100644 --- a/crates/apub_receive/src/activities/private_message/delete.rs +++ b/crates/apub_receive/src/activities/private_message/delete.rs @@ -1,4 +1,4 @@ -use crate::activities::private_message::{send_websocket_message, verify_activity, verify_person}; +use crate::activities::{private_message::send_websocket_message, verify_activity, verify_person}; use activitystreams::activity::kind::DeleteType; use lemmy_api_common::blocking; use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew}; diff --git a/crates/apub_receive/src/activities/private_message/mod.rs b/crates/apub_receive/src/activities/private_message/mod.rs index 12378b5e7..beb28b299 100644 --- a/crates/apub_receive/src/activities/private_message/mod.rs +++ b/crates/apub_receive/src/activities/private_message/mod.rs @@ -1,38 +1,14 @@ -use anyhow::anyhow; use lemmy_api_common::{blocking, person::PrivateMessageResponse}; -use lemmy_apub::{check_is_apub_id_valid, fetcher::person::get_or_fetch_and_upsert_person}; -use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields}; use lemmy_db_schema::PrivateMessageId; use lemmy_db_views::{local_user_view::LocalUserView, private_message_view::PrivateMessageView}; use lemmy_utils::LemmyError; use lemmy_websocket::{messages::SendUserRoomMessage, LemmyContext, UserOperationCrud}; -use url::Url; pub mod create; pub mod delete; pub mod undo_delete; pub mod update; -/// Checks that the specified Url actually identifies a Person (by fetching it), and that the person -/// doesn't have a site ban. -async fn verify_person( - person_id: &Url, - context: &LemmyContext, - request_counter: &mut i32, -) -> Result<(), LemmyError> { - let person = get_or_fetch_and_upsert_person(person_id, context, request_counter).await?; - if person.banned { - return Err(anyhow!("Person {} is banned", person_id).into()); - } - Ok(()) -} - -fn verify_activity(common: &ActivityCommonFields) -> Result<(), LemmyError> { - check_is_apub_id_valid(&common.actor, false)?; - verify_domains_match(common.id_unchecked(), &common.actor)?; - Ok(()) -} - async fn send_websocket_message( private_message_id: PrivateMessageId, op: UserOperationCrud, diff --git a/crates/apub_receive/src/activities/private_message/undo_delete.rs b/crates/apub_receive/src/activities/private_message/undo_delete.rs index 3581233af..3c9298d73 100644 --- a/crates/apub_receive/src/activities/private_message/undo_delete.rs +++ b/crates/apub_receive/src/activities/private_message/undo_delete.rs @@ -1,6 +1,5 @@ -use crate::activities::private_message::{ - delete::DeletePrivateMessage, - send_websocket_message, +use crate::activities::{ + private_message::{delete::DeletePrivateMessage, send_websocket_message}, verify_activity, verify_person, }; diff --git a/crates/apub_receive/src/activities/private_message/update.rs b/crates/apub_receive/src/activities/private_message/update.rs index 19633ce60..ea716dfa8 100644 --- a/crates/apub_receive/src/activities/private_message/update.rs +++ b/crates/apub_receive/src/activities/private_message/update.rs @@ -1,4 +1,4 @@ -use crate::activities::private_message::{send_websocket_message, verify_activity, verify_person}; +use crate::activities::{private_message::send_websocket_message, verify_activity, verify_person}; use activitystreams::{activity::kind::UpdateType, base::BaseExt}; use lemmy_apub::{objects::FromApub, NoteExt}; use lemmy_apub_lib::{verify_domains_match_opt, ActivityCommonFields, ActivityHandlerNew};