Merge pull request 'Add user_inbox check that activities are really addressed to local users' (#131) from user-inbox-check into main

Reviewed-on: https://yerbamate.ml/LemmyNet/lemmy/pulls/131

lemmy_apub/src/inbox/mod.rs

@@ -12,7 +12,7 @@ use activitystreams::{
 };
 use actix_web::HttpRequest;
 use anyhow::{anyhow, Context};
-use lemmy_db::{activity::Activity, DbPool};
+use lemmy_db::{activity::Activity, community::Community, user::User_, DbPool};
 use lemmy_structs::blocking;
 use lemmy_utils::{location_info, LemmyError};
 use lemmy_websocket::LemmyContext;
@@ -111,3 +111,43 @@ where
   verify_signature(&request, actor.as_ref())?;
   Ok(actor)
 }
+
+/// Returns true if `to_and_cc` contains at least one local user.
+pub(crate) async fn is_addressed_to_local_user(
+  to_and_cc: &[Url],
+  pool: &DbPool,
+) -> Result<bool, LemmyError> {
+  for url in to_and_cc {
+    let url = url.to_string();
+    let user = blocking(&pool, move |conn| User_::read_from_actor_id(&conn, &url)).await?;
+    if let Ok(u) = user {
+      if u.local {
+        return Ok(true);
+      }
+    }
+  }
+  Ok(false)
+}
+
+/// If `to_and_cc` contains the followers collection of a remote community, returns this community
+/// (like `https://example.com/c/main/followers`)
+pub(crate) async fn is_addressed_to_community_followers(
+  to_and_cc: &[Url],
+  pool: &DbPool,
+) -> Result<Option<Community>, LemmyError> {
+  for url in to_and_cc {
+    let url = url.to_string();
+    // TODO: extremely hacky, we should just store the followers url for each community in the db
+    if url.ends_with("/followers") {
+      let community_url = url.replace("/followers", "");
+      let community = blocking(&pool, move |conn| {
+        Community::read_from_actor_id(&conn, &community_url)
+      })
+      .await??;
+      if !community.local {
+        return Ok(Some(community));
+      }
+    }
+  }
+  Ok(None)
+}

lemmy_apub/src/inbox/shared_inbox.rs

@@ -5,6 +5,8 @@ use crate::{
     get_activity_to_and_cc,
     inbox_verify_http_signature,
     is_activity_already_known,
+    is_addressed_to_community_followers,
+    is_addressed_to_local_user,
     user_inbox::{user_receive_message, UserAcceptedActivities},
   },
   insert_activity,
@@ -12,7 +14,7 @@ use crate::{
 use activitystreams::{activity::ActorAndObject, prelude::*};
 use actix_web::{web, HttpRequest, HttpResponse};
 use anyhow::Context;
-use lemmy_db::{community::Community, user::User_, DbPool};
+use lemmy_db::{community::Community, DbPool};
 use lemmy_structs::blocking;
 use lemmy_utils::{location_info, LemmyError};
 use lemmy_websocket::LemmyContext;
@@ -100,7 +102,10 @@ pub async fn shared_inbox(
   }
 
   // If to_and_cc contains followers collection of a community, pass to receive_user_message()
-  if is_addressed_to_community_followers(&to_and_cc, context.pool()).await? {
+  if is_addressed_to_community_followers(&to_and_cc, context.pool())
+    .await?
+    .is_some()
+  {
     let user_activity = UserAcceptedActivities::from_any_base(activity_any_base.clone())?
       .context(location_info!())?;
     res = Some(
@@ -146,40 +151,3 @@ async fn extract_local_community_from_destinations(
   }
   Ok(None)
 }
-
-/// Returns true if `to_and_cc` contains at least one local user.
-async fn is_addressed_to_local_user(to_and_cc: &[Url], pool: &DbPool) -> Result<bool, LemmyError> {
-  for url in to_and_cc {
-    let url = url.to_string();
-    let user = blocking(&pool, move |conn| User_::read_from_actor_id(&conn, &url)).await?;
-    if let Ok(u) = user {
-      if u.local {
-        return Ok(true);
-      }
-    }
-  }
-  Ok(false)
-}
-
-/// Returns true if `to_and_cc` contains at least one followers collection of a remote community
-/// (like `https://example.com/c/main/followers`)
-async fn is_addressed_to_community_followers(
-  to_and_cc: &[Url],
-  pool: &DbPool,
-) -> Result<bool, LemmyError> {
-  for url in to_and_cc {
-    let url = url.to_string();
-    // TODO: extremely hacky, we should just store the followers url for each community in the db
-    if url.ends_with("/followers") {
-      let community_url = url.replace("/followers", "");
-      let community = blocking(&pool, move |conn| {
-        Community::read_from_actor_id(&conn, &community_url)
-      })
-      .await??;
-      if !community.local {
-        return Ok(true);
-      }
-    }
-  }
-  Ok(false)
-}

lemmy_apub/src/inbox/user_inbox.rs

@@ -23,6 +23,8 @@ use crate::{
     get_activity_to_and_cc,
     inbox_verify_http_signature,
     is_activity_already_known,
+    is_addressed_to_community_followers,
+    is_addressed_to_local_user,
     is_addressed_to_public,
     receive_for_community::{
       receive_create_for_community,
@@ -99,6 +101,7 @@ pub async fn user_inbox(
   })
   .await??;
   let to_and_cc = get_activity_to_and_cc(&activity)?;
+  // TODO: we should also accept activities that are sent to community followers
   if !to_and_cc.contains(&&user.actor_id()?) {
     return Err(anyhow!("Activity delivered to wrong user").into());
   }
@@ -130,9 +133,7 @@ pub(crate) async fn user_receive_message(
   context: &LemmyContext,
   request_counter: &mut i32,
 ) -> Result<HttpResponse, LemmyError> {
-  // TODO: must be addressed to one or more local users, or to followers of a remote community
-
-  // TODO: if it is addressed to community followers, check that at least one local user is following it
+  is_for_user_inbox(context, &activity).await?;
 
   let any_base = activity.clone().into_any_base()?;
   let kind = activity.kind().context(location_info!())?;
@@ -162,6 +163,41 @@ pub(crate) async fn user_receive_message(
   Ok(HttpResponse::Ok().finish())
 }
 
+/// Returns true if the activity is addressed directly to one or more local users, or if it is
+/// addressed to the followers collection of a remote community, and at least one local user follows
+/// it.
+async fn is_for_user_inbox(
+  context: &LemmyContext,
+  activity: &UserAcceptedActivities,
+) -> Result<(), LemmyError> {
+  let to_and_cc = get_activity_to_and_cc(activity)?;
+  // Check if it is addressed directly to any local user
+  if is_addressed_to_local_user(&to_and_cc, context.pool()).await? {
+    return Ok(());
+  }
+
+  // Check if it is addressed to any followers collection of a remote community, and that the
+  // community has local followers.
+  let community = is_addressed_to_community_followers(&to_and_cc, context.pool()).await?;
+  if let Some(c) = community {
+    let community_id = c.id;
+    let has_local_followers = blocking(&context.pool(), move |conn| {
+      CommunityFollower::has_local_followers(conn, community_id)
+    })
+    .await??;
+    if c.local {
+      return Err(
+        anyhow!("Remote activity cant be addressed to followers of local community").into(),
+      );
+    }
+    if has_local_followers {
+      return Ok(());
+    }
+  }
+
+  Err(anyhow!("Not addressed for any local user").into())
+}
+
 /// Handle accepted follows.
 async fn receive_accept(
   context: &LemmyContext,

lemmy_db/src/community.rs

@@ -322,6 +322,15 @@ impl Followable<CommunityFollowerForm> for CommunityFollower {
     )
     .execute(conn)
   }
+  // TODO: this function name only makes sense if you call it with a remote community. for a local
+  //       community, it will also return true if only remote followers exist
+  fn has_local_followers(conn: &PgConnection, community_id_: i32) -> Result<bool, Error> {
+    use crate::schema::community_follower::dsl::*;
+    diesel::select(exists(
+      community_follower.filter(community_id.eq(community_id_)),
+    ))
+    .get_result(conn)
+  }
 }
 
 #[cfg(test)]

lemmy_db/src/lib.rs

@@ -62,6 +62,7 @@ pub trait Followable<T> {
   fn unfollow(conn: &PgConnection, form: &T) -> Result<usize, Error>
   where
     Self: Sized;
+  fn has_local_followers(conn: &PgConnection, community_id: i32) -> Result<bool, Error>;
 }
 
 pub trait Joinable<T> {