rework following verify(), fix tests and test manually

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,

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(

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(

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(

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,

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};

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};

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,

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,
 };

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};