rework pm verify(), fix tests and confirm manually

also remove inbox username check which was broken

crates/apub_lib/src/lib.rs

@@ -81,6 +81,20 @@ pub fn verify_domains_match(a: &Url, b: &Url) -> Result<(), LemmyError> {
   Ok(())
 }
 
+pub fn verify_domains_match_opt(a: &Url, b: Option<&Url>) -> Result<(), LemmyError> {
+  if let Some(b2) = b {
+    return verify_domains_match(a, b2);
+  }
+  Ok(())
+}
+
+pub fn verify_urls_match(a: &Url, b: &Url) -> Result<(), LemmyError> {
+  if a != b {
+    return Err(DomainError.into());
+  }
+  Ok(())
+}
+
 // todo: instead of phantomdata, might use option<kind> to cache the fetched object (or just fetch on construction)
 pub struct ObjectId<'a, Kind>(Url, &'a PhantomData<Kind>);
 

crates/apub_receive/src/activities/private_message/create.rs

@@ -1,7 +1,7 @@
-use crate::activities::private_message::send_websocket_message;
+use crate::activities::private_message::{send_websocket_message, verify_activity, verify_person};
 use activitystreams::{activity::kind::CreateType, base::BaseExt};
-use lemmy_apub::{check_is_apub_id_valid, objects::FromApub, NoteExt};
+use lemmy_apub::{objects::FromApub, NoteExt};
-use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew};
+use lemmy_apub_lib::{verify_domains_match_opt, ActivityCommonFields, ActivityHandlerNew};
 use lemmy_db_schema::source::private_message::PrivateMessage;
 use lemmy_utils::LemmyError;
 use lemmy_websocket::{LemmyContext, UserOperationCrud};
@@ -20,10 +20,15 @@ pub struct CreatePrivateMessage {
 
 #[async_trait::async_trait(?Send)]
 impl ActivityHandlerNew for CreatePrivateMessage {
-  async fn verify(&self, _context: &LemmyContext, _: &mut i32) -> Result<(), LemmyError> {
+  async fn verify(
-    verify_domains_match(self.common.id_unchecked(), &self.common.actor)?;
+    &self,
-    self.object.id(self.common.actor.as_str())?;
+    context: &LemmyContext,
-    check_is_apub_id_valid(&self.common.actor, false)
+    request_counter: &mut i32,
+  ) -> Result<(), LemmyError> {
+    verify_activity(self.common())?;
+    verify_person(&self.common.actor, context, request_counter).await?;
+    verify_domains_match_opt(&self.common.actor, self.object.id_unchecked())?;
+    Ok(())
   }
 
   async fn receive(

crates/apub_receive/src/activities/private_message/delete.rs

@@ -1,7 +1,6 @@
-use crate::activities::private_message::send_websocket_message;
+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::check_is_apub_id_valid;
 use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew};
 use lemmy_db_queries::{source::private_message::PrivateMessage_, ApubObject};
 use lemmy_db_schema::source::private_message::PrivateMessage;
@@ -22,10 +21,15 @@ pub struct DeletePrivateMessage {
 
 #[async_trait::async_trait(?Send)]
 impl ActivityHandlerNew for DeletePrivateMessage {
-  async fn verify(&self, _context: &LemmyContext, _: &mut i32) -> Result<(), LemmyError> {
+  async fn verify(
-    verify_domains_match(&self.common.actor, self.common.id_unchecked())?;
+    &self,
+    context: &LemmyContext,
+    request_counter: &mut i32,
+  ) -> Result<(), LemmyError> {
+    verify_activity(self.common())?;
+    verify_person(&self.common.actor, context, request_counter).await?;
     verify_domains_match(&self.common.actor, &self.object)?;
-    check_is_apub_id_valid(&self.common.actor, false)
+    Ok(())
   }
 
   async fn receive(

crates/apub_receive/src/activities/private_message/mod.rs

@@ -1,14 +1,38 @@
+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,8 +1,17 @@
-use crate::activities::private_message::{delete::DeletePrivateMessage, send_websocket_message};
+use crate::activities::private_message::{
+  delete::DeletePrivateMessage,
+  send_websocket_message,
+  verify_activity,
+  verify_person,
+};
 use activitystreams::activity::kind::UndoType;
 use lemmy_api_common::blocking;
-use lemmy_apub::check_is_apub_id_valid;
+use lemmy_apub_lib::{
-use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew};
+  verify_domains_match,
+  verify_urls_match,
+  ActivityCommonFields,
+  ActivityHandlerNew,
+};
 use lemmy_db_queries::{source::private_message::PrivateMessage_, ApubObject};
 use lemmy_db_schema::source::private_message::PrivateMessage;
 use lemmy_utils::LemmyError;
@@ -27,10 +36,12 @@ impl ActivityHandlerNew for UndoDeletePrivateMessage {
     context: &LemmyContext,
     request_counter: &mut i32,
   ) -> Result<(), LemmyError> {
-    verify_domains_match(&self.common.actor, self.common.id_unchecked())?;
+    verify_activity(self.common())?;
-    verify_domains_match(&self.common.actor, self.object.common.id_unchecked())?;
+    verify_person(&self.common.actor, context, request_counter).await?;
-    check_is_apub_id_valid(&self.common.actor, false)?;
+    verify_urls_match(&self.common.actor, &self.object.common.actor)?;
-    self.object.verify(context, request_counter).await
+    verify_domains_match(&self.common.actor, &self.object.object)?;
+    self.object.verify(context, request_counter).await?;
+    Ok(())
   }
 
   async fn receive(

crates/apub_receive/src/activities/private_message/update.rs

@@ -1,7 +1,7 @@
-use crate::activities::private_message::send_websocket_message;
+use crate::activities::private_message::{send_websocket_message, verify_activity, verify_person};
 use activitystreams::{activity::kind::UpdateType, base::BaseExt};
-use lemmy_apub::{check_is_apub_id_valid, objects::FromApub, NoteExt};
+use lemmy_apub::{objects::FromApub, NoteExt};
-use lemmy_apub_lib::{verify_domains_match, ActivityCommonFields, ActivityHandlerNew};
+use lemmy_apub_lib::{verify_domains_match_opt, ActivityCommonFields, ActivityHandlerNew};
 use lemmy_db_schema::source::private_message::PrivateMessage;
 use lemmy_utils::LemmyError;
 use lemmy_websocket::{LemmyContext, UserOperationCrud};
@@ -20,10 +20,15 @@ pub struct UpdatePrivateMessage {
 
 #[async_trait::async_trait(?Send)]
 impl ActivityHandlerNew for UpdatePrivateMessage {
-  async fn verify(&self, _context: &LemmyContext, _: &mut i32) -> Result<(), LemmyError> {
+  async fn verify(
-    verify_domains_match(self.common.id_unchecked(), &self.common.actor)?;
+    &self,
-    self.object.id(self.common.actor.as_str())?;
+    context: &LemmyContext,
-    check_is_apub_id_valid(&self.common.actor, false)
+    request_counter: &mut i32,
+  ) -> Result<(), LemmyError> {
+    verify_activity(self.common())?;
+    verify_person(&self.common.actor, context, request_counter).await?;
+    verify_domains_match_opt(&self.common.actor, self.object.id_unchecked())?;
+    Ok(())
   }
 
   async fn receive(

crates/apub_receive/src/http/community.rs

@@ -56,11 +56,11 @@ pub(crate) async fn get_apub_community_http(
 pub async fn community_inbox(
   request: HttpRequest,
   payload: Payload,
-  path: web::Path<String>,
+  _path: web::Path<String>,
   context: web::Data<LemmyContext>,
 ) -> Result<HttpResponse, LemmyError> {
   let unparsed = payload_to_string(payload).await?;
-  receive_activity::<GroupInboxActivities>(request, &unparsed, Some(path.0), context).await
+  receive_activity::<GroupInboxActivities>(request, &unparsed, context).await
 }
 
 /// Returns an empty followers collection, only populating the size (for privacy).

crates/apub_receive/src/http/mod.rs

@@ -50,7 +50,7 @@ pub async fn shared_inbox(
   context: web::Data<LemmyContext>,
 ) -> Result<HttpResponse, LemmyError> {
   let unparsed = payload_to_string(payload).await?;
-  receive_activity::<Ac>(request, &unparsed, None, context).await
+  receive_activity::<Ac>(request, &unparsed, context).await
 }
 
 async fn payload_to_string(mut payload: Payload) -> Result<String, LemmyError> {
@@ -65,7 +65,6 @@ async fn payload_to_string(mut payload: Payload) -> Result<String, LemmyError> {
 async fn receive_activity<'a, T>(
   request: HttpRequest,
   activity: &'a str,
-  expected_name: Option<String>,
   context: web::Data<LemmyContext>,
 ) -> Result<HttpResponse, LemmyError>
 where
@@ -85,11 +84,6 @@ where
   let request_counter = &mut 0;
   let actor =
     get_or_fetch_and_upsert_actor(&activity_data.actor, &context, request_counter).await?;
-  if let Some(expected) = expected_name {
-    if expected != actor.name() {
-      return Ok(HttpResponse::BadRequest().finish());
-    }
-  }
   verify_signature(&request, &actor.public_key().context(location_info!())?)?;
   activity.verify(&context, request_counter).await?;
 

crates/apub_receive/src/http/person.rs

@@ -48,11 +48,11 @@ pub(crate) async fn get_apub_person_http(
 pub async fn person_inbox(
   request: HttpRequest,
   payload: Payload,
-  path: web::Path<String>,
+  _path: web::Path<String>,
   context: web::Data<LemmyContext>,
 ) -> Result<HttpResponse, LemmyError> {
   let unparsed = payload_to_string(payload).await?;
-  receive_activity::<PersonInboxActivities>(request, &unparsed, Some(path.0), context).await
+  receive_activity::<PersonInboxActivities>(request, &unparsed, context).await
 }
 
 pub(crate) async fn get_apub_person_outbox(