More test coverage for private community, fix some bugs (#5207)

* More test coverage for private community, fix some bugs

* fmt

* stuff

* attempted fix

* fix tests

* api tests passing

* fix tests

* apub lib

Cargo.lock

@@ -10,9 +10,9 @@ checksum = "8f27d075294830fcab6f66e320dab524bc6d048f4a151698e153205559113772"
 
 [[package]]
 name = "activitypub_federation"
-version = "0.6.0-alpha2"
+version = "0.6.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4877d467ddf2fac85e9ee33aba6f2560df14125b8bfa864f85ab40e9b87753a9"
+checksum = "ee819cada736b6e26c59706f9e6ff89a48060e635c0546ff984d84baefc8c13a"
 dependencies = [
  "activitystreams-kinds",
  "actix-web",

Cargo.toml

@@ -94,7 +94,7 @@ lemmy_db_views = { version = "=0.19.6-beta.7", path = "./crates/db_views" }
 lemmy_db_views_actor = { version = "=0.19.6-beta.7", path = "./crates/db_views_actor" }
 lemmy_db_views_moderator = { version = "=0.19.6-beta.7", path = "./crates/db_views_moderator" }
 lemmy_federate = { version = "=0.19.6-beta.7", path = "./crates/federate" }
-activitypub_federation = { version = "0.6.0-alpha2", default-features = false, features = [
+activitypub_federation = { version = "0.6.1", default-features = false, features = [
   "actix-web",
 ] }
 diesel = "2.2.4"

api_tests/src/private_community.spec.ts

@@ -1,6 +1,6 @@
 jest.setTimeout(120000);
 
-import { FollowCommunity } from "lemmy-js-client";
+import { FollowCommunity, LemmyHttp } from "lemmy-js-client";
 import {
   alpha,
   setupLogins,
@@ -21,6 +21,9 @@ import {
   resolveComment,
   likeComment,
   waitUntil,
+  gamma,
+  getPosts,
+  getComments,
 } from "./shared";
 
 beforeAll(setupLogins);
@@ -47,6 +50,7 @@ test("Follow a private community", async () => {
     await resolveCommunity(user, community.community_view.community.actor_id)
   ).community;
   expect(betaCommunity).toBeDefined();
+  expect(betaCommunity?.community.visibility).toBe("Private");
   const betaCommunityId = betaCommunity!.community.id;
   const follow_form: FollowCommunity = {
     community_id: betaCommunityId,
@@ -148,16 +152,7 @@ test("Only followers can view and interact with private community content", asyn
     follow: true,
   };
   await user.followCommunity(follow_form);
-  const pendingFollows1 = await waitUntil(
+  approveFollower(alpha, alphaCommunityId);
-    () => listCommunityPendingFollows(alpha),
-    f => f.items.length == 1,
-  );
-  const approve = await approveCommunityPendingFollow(
-    alpha,
-    alphaCommunityId,
-    pendingFollows1.items[0].person.id,
-  );
-  expect(approve.success).toBe(true);
 
   // now user can fetch posts and comments in community (using signed fetch), and create posts
   await waitUntil(
@@ -212,3 +207,151 @@ test("Reject follower", async () => {
     c => c.community_view.subscribed == "NotSubscribed",
   );
 });
+
+test("Follow a private community and receive activities", async () => {
+  // create private community
+  const community = await createCommunity(alpha, randomString(10), "Private");
+  expect(community.community_view.community.visibility).toBe("Private");
+  const alphaCommunityId = community.community_view.community.id;
+
+  // follow with users from beta and gamma
+  const betaCommunity = (
+    await resolveCommunity(beta, community.community_view.community.actor_id)
+  ).community;
+  expect(betaCommunity).toBeDefined();
+  const betaCommunityId = betaCommunity!.community.id;
+  const follow_form_beta: FollowCommunity = {
+    community_id: betaCommunityId,
+    follow: true,
+  };
+  await beta.followCommunity(follow_form_beta);
+  await approveFollower(alpha, alphaCommunityId);
+
+  const gammaCommunityId = (
+    await resolveCommunity(gamma, community.community_view.community.actor_id)
+  ).community!.community.id;
+  const follow_form_gamma: FollowCommunity = {
+    community_id: gammaCommunityId,
+    follow: true,
+  };
+  await gamma.followCommunity(follow_form_gamma);
+  await approveFollower(alpha, alphaCommunityId);
+
+  // Follow is confirmed
+  await waitUntil(
+    () => getCommunity(beta, betaCommunityId),
+    c => c.community_view.subscribed == "Subscribed",
+  );
+  await waitUntil(
+    () => getCommunity(gamma, gammaCommunityId),
+    c => c.community_view.subscribed == "Subscribed",
+  );
+
+  // create a post and comment from gamma
+  const post = await createPost(gamma, gammaCommunityId);
+  const post_id = post.post_view.post.id;
+  expect(post_id).toBeDefined();
+  const comment = await createComment(gamma, post_id);
+  const comment_id = comment.comment_view.comment.id;
+  expect(comment_id).toBeDefined();
+
+  // post and comment were federated to beta
+  let posts = await waitUntil(
+    () => getPosts(beta, "All", betaCommunityId),
+    c => c.posts.length == 1,
+  );
+  expect(posts.posts[0].post.ap_id).toBe(post.post_view.post.ap_id);
+  expect(posts.posts[0].post.name).toBe(post.post_view.post.name);
+  let comments = await waitUntil(
+    () => getComments(beta, posts.posts[0].post.id),
+    c => c.comments.length == 1,
+  );
+  expect(comments.comments[0].comment.ap_id).toBe(
+    comment.comment_view.comment.ap_id,
+  );
+  expect(comments.comments[0].comment.content).toBe(
+    comment.comment_view.comment.content,
+  );
+});
+
+test("Fetch remote content in private community", async () => {
+  // create private community
+  const community = await createCommunity(alpha, randomString(10), "Private");
+  expect(community.community_view.community.visibility).toBe("Private");
+  const alphaCommunityId = community.community_view.community.id;
+
+  const betaCommunityId = (
+    await resolveCommunity(beta, community.community_view.community.actor_id)
+  ).community!.community.id;
+  const follow_form_beta: FollowCommunity = {
+    community_id: betaCommunityId,
+    follow: true,
+  };
+  await beta.followCommunity(follow_form_beta);
+  await approveFollower(alpha, alphaCommunityId);
+
+  // Follow is confirmed
+  await waitUntil(
+    () => getCommunity(beta, betaCommunityId),
+    c => c.community_view.subscribed == "Subscribed",
+  );
+
+  // beta creates post and comment
+  const post = await createPost(beta, betaCommunityId);
+  const post_id = post.post_view.post.id;
+  expect(post_id).toBeDefined();
+  const comment = await createComment(beta, post_id);
+  const comment_id = comment.comment_view.comment.id;
+  expect(comment_id).toBeDefined();
+
+  // Wait for it to federate
+  await waitUntil(
+    () => resolveComment(alpha, comment.comment_view.comment),
+    p => p?.comment?.comment.id != undefined,
+  );
+
+  // create gamma user
+  const gammaCommunityId = (
+    await resolveCommunity(gamma, community.community_view.community.actor_id)
+  ).community!.community.id;
+  const follow_form: FollowCommunity = {
+    community_id: gammaCommunityId,
+    follow: true,
+  };
+
+  // cannot fetch post yet
+  await expect(resolvePost(gamma, post.post_view.post)).rejects.toStrictEqual(
+    Error("not_found"),
+  );
+  // follow community and approve
+  await gamma.followCommunity(follow_form);
+  await approveFollower(alpha, alphaCommunityId);
+
+  // now user can fetch posts and comments in community (using signed fetch), and create posts.
+  // for this to work, beta checks with alpha if gamma is really an approved follower.
+  let resolvedPost = await waitUntil(
+    () => resolvePost(gamma, post.post_view.post),
+    p => p?.post?.post.id != undefined,
+  );
+  expect(resolvedPost.post?.post.ap_id).toBe(post.post_view.post.ap_id);
+  const resolvedComment = await waitUntil(
+    () => resolveComment(gamma, comment.comment_view.comment),
+    p => p?.comment?.comment.id != undefined,
+  );
+  expect(resolvedComment?.comment?.comment.ap_id).toBe(
+    comment.comment_view.comment.ap_id,
+  );
+});
+
+async function approveFollower(user: LemmyHttp, community_id: number) {
+  let pendingFollows1 = await waitUntil(
+    () => listCommunityPendingFollows(user),
+    f => f.items.length == 1,
+  );
+  const approve = await approveCommunityPendingFollow(
+    alpha,
+    community_id,
+    pendingFollows1.items[0].person.id,
+  );
+  expect(approve.success).toBe(true);
+}

crates/api_common/src/build_response.rs

@@ -17,8 +17,10 @@ use lemmy_db_schema::{
     actor_language::CommunityLanguage,
     comment::Comment,
     comment_reply::{CommentReply, CommentReplyInsertForm},
+    community::Community,
     person::Person,
     person_mention::{PersonMention, PersonMentionInsertForm},
+    post::Post,
   },
   traits::Crud,
 };
@@ -101,17 +103,28 @@ pub async fn send_local_notifs(
   let mut recipient_ids = Vec::new();
   let inbox_link = format!("{}/inbox", context.settings().get_protocol_and_hostname());
 
-  // let person = my_local_user.person;
+  // When called from api code, we have local user view and can read with CommentView
-  // Read the comment view to get extra info
+  // to reduce db queries. But when receiving a federated comment the user view is None,
+  // which means that comments inside private communities cant be read. As a workaround
+  // we need to read the items manually to bypass this check.
+  let (comment, post, community) = if let Some(local_user_view) = local_user_view {
     let comment_view = CommentView::read(
       &mut context.pool(),
       comment_id,
-    local_user_view.map(|view| &view.local_user),
+      Some(&local_user_view.local_user),
     )
     .await?;
-  let comment = comment_view.comment;
+    (
-  let post = comment_view.post;
+      comment_view.comment,
-  let community = comment_view.community;
+      comment_view.post,
+      comment_view.community,
+    )
+  } else {
+    let comment = Comment::read(&mut context.pool(), comment_id).await?;
+    let post = Post::read(&mut context.pool(), comment.post_id).await?;
+    let community = Community::read(&mut context.pool(), post.community_id).await?;
+    (comment, post, community)
+  };
 
   // Send the local mentions
   for mention in mentions

crates/apub/src/activities/community/announce.rs

@@ -215,7 +215,7 @@ async fn can_accept_activity_in_community(
 ) -> LemmyResult<()> {
   if let Some(community) = community {
     // Local only community can't federate
-    if community.visibility != CommunityVisibility::Public {
+    if community.visibility == CommunityVisibility::LocalOnly {
       return Err(LemmyErrorType::NotFound.into());
     }
     if !community.local {

crates/apub/src/activities/community/mod.rs

@@ -42,7 +42,7 @@ pub(crate) async fn send_activity_in_community(
   context: &Data<LemmyContext>,
 ) -> LemmyResult<()> {
   // If community is local only, don't send anything out
-  if community.visibility != CommunityVisibility::Public {
+  if community.visibility == CommunityVisibility::LocalOnly {
     return Ok(());
   }
 

crates/apub/src/activities/create_or_update/comment.rs

@@ -171,6 +171,9 @@ impl ActivityHandler for CreateOrUpdateNote {
     // TODO: for compatibility with other projects, it would be much better to read this from cc or
     // tags
     let mentions = scrape_text_for_mentions(&comment.content);
+
+    // TODO: this fails in local community comment as CommentView::read() returns nothing
+    //       without passing LocalUser
     send_local_notifs(mentions, comment.id, &actor, do_send_email, context, None).await?;
     Ok(())
   }

crates/apub/src/http/community.rs

@@ -6,28 +6,41 @@ use crate::{
     community_moderators::ApubCommunityModerators,
     community_outbox::ApubCommunityOutbox,
   },
+  fetcher::site_or_community_or_user::SiteOrCommunityOrUser,
   http::{check_community_fetchable, create_apub_response, create_apub_tombstone_response},
   objects::community::ApubCommunity,
 };
 use activitypub_federation::{
+  actix_web::signing_actor,
   config::Data,
+  fetch::object_id::ObjectId,
   traits::{Collection, Object},
 };
-use actix_web::{web, HttpRequest, HttpResponse};
+use actix_web::{
+  web::{Path, Query},
+  HttpRequest,
+  HttpResponse,
+};
 use lemmy_api_common::context::LemmyContext;
-use lemmy_db_schema::{source::community::Community, traits::ApubActor};
+use lemmy_db_schema::{source::community::Community, traits::ApubActor, CommunityVisibility};
+use lemmy_db_views_actor::structs::CommunityFollowerView;
 use lemmy_utils::error::{LemmyErrorType, LemmyResult};
 use serde::Deserialize;
 
 #[derive(Deserialize, Clone)]
-pub(crate) struct CommunityQuery {
+pub(crate) struct CommunityPath {
   community_name: String,
 }
 
+#[derive(Deserialize, Clone)]
+pub struct CommunityIsFollowerQuery {
+  is_follower: Option<ObjectId<SiteOrCommunityOrUser>>,
+}
+
 /// Return the ActivityPub json representation of a local community over HTTP.
 #[tracing::instrument(skip_all)]
 pub(crate) async fn get_apub_community_http(
-  info: web::Path<CommunityQuery>,
+  info: Path<CommunityPath>,
   context: Data<LemmyContext>,
 ) -> LemmyResult<HttpResponse> {
   let community: ApubCommunity =
@@ -47,21 +60,59 @@ pub(crate) async fn get_apub_community_http(
 
 /// Returns an empty followers collection, only populating the size (for privacy).
 pub(crate) async fn get_apub_community_followers(
-  info: web::Path<CommunityQuery>,
+  info: Path<CommunityPath>,
+  query: Query<CommunityIsFollowerQuery>,
   context: Data<LemmyContext>,
+  request: HttpRequest,
 ) -> LemmyResult<HttpResponse> {
   let community = Community::read_from_name(&mut context.pool(), &info.community_name, false)
     .await?
     .ok_or(LemmyErrorType::NotFound)?;
+  if let Some(is_follower) = &query.is_follower {
+    return check_is_follower(community, is_follower, context, request).await;
+  }
   check_community_fetchable(&community)?;
   let followers = ApubCommunityFollower::read_local(&community.into(), &context).await?;
   create_apub_response(&followers)
 }
 
+/// Checks if a given actor follows the private community. Returns status 200 if true.
+async fn check_is_follower(
+  community: Community,
+  is_follower: &ObjectId<SiteOrCommunityOrUser>,
+  context: Data<LemmyContext>,
+  request: HttpRequest,
+) -> LemmyResult<HttpResponse> {
+  if community.visibility != CommunityVisibility::Private {
+    return Ok(HttpResponse::BadRequest().body("must be a private community"));
+  }
+  // also check for http sig so that followers are not exposed publicly
+  let signing_actor = signing_actor::<SiteOrCommunityOrUser>(&request, None, &context).await?;
+  CommunityFollowerView::check_has_followers_from_instance(
+    community.id,
+    signing_actor.instance_id(),
+    &mut context.pool(),
+  )
+  .await?;
+
+  let instance_id = is_follower.dereference(&context).await?.instance_id();
+  let has_followers = CommunityFollowerView::check_has_followers_from_instance(
+    community.id,
+    instance_id,
+    &mut context.pool(),
+  )
+  .await;
+  if has_followers.is_ok() {
+    Ok(HttpResponse::Ok().finish())
+  } else {
+    Ok(HttpResponse::NotFound().finish())
+  }
+}
+
 /// Returns the community outbox, which is populated by a maximum of 20 posts (but no other
 /// activities like votes or comments).
 pub(crate) async fn get_apub_community_outbox(
-  info: web::Path<CommunityQuery>,
+  info: Path<CommunityPath>,
   context: Data<LemmyContext>,
   request: HttpRequest,
 ) -> LemmyResult<HttpResponse> {
@@ -77,7 +128,7 @@ pub(crate) async fn get_apub_community_outbox(
 
 #[tracing::instrument(skip_all)]
 pub(crate) async fn get_apub_community_moderators(
-  info: web::Path<CommunityQuery>,
+  info: Path<CommunityPath>,
   context: Data<LemmyContext>,
 ) -> LemmyResult<HttpResponse> {
   let community: ApubCommunity =
@@ -92,7 +143,7 @@ pub(crate) async fn get_apub_community_moderators(
 
 /// Returns collection of featured (stickied) posts.
 pub(crate) async fn get_apub_community_featured(
-  info: web::Path<CommunityQuery>,
+  info: Path<CommunityPath>,
   context: Data<LemmyContext>,
   request: HttpRequest,
 ) -> LemmyResult<HttpResponse> {
@@ -181,17 +232,17 @@ pub(crate) mod tests {
     let request = TestRequest::default().to_http_request();
 
     // fetch invalid community
-    let query = CommunityQuery {
+    let query = CommunityPath {
       community_name: "asd".to_string(),
     };
     let res = get_apub_community_http(query.into(), context.reset_request_count()).await;
     assert!(res.is_err());
 
     // fetch valid community
-    let query = CommunityQuery {
+    let path = CommunityPath {
       community_name: community.name.clone(),
     };
-    let res = get_apub_community_http(query.clone().into(), context.reset_request_count()).await?;
+    let res = get_apub_community_http(path.clone().into(), context.reset_request_count()).await?;
     assert_eq!(200, res.status());
     let res_group: Group = decode_response(res).await?;
     let community: ApubCommunity = community.into();
@@ -199,20 +250,26 @@ pub(crate) mod tests {
     assert_eq!(group, res_group);
 
     let res = get_apub_community_featured(
-      query.clone().into(),
+      path.clone().into(),
+      context.reset_request_count(),
+      request.clone(),
+    )
+    .await?;
+    assert_eq!(200, res.status());
+    let query = Query(CommunityIsFollowerQuery { is_follower: None });
+    let res = get_apub_community_followers(
+      path.clone().into(),
+      query,
       context.reset_request_count(),
       request.clone(),
     )
     .await?;
     assert_eq!(200, res.status());
     let res =
-      get_apub_community_followers(query.clone().into(), context.reset_request_count()).await?;
+      get_apub_community_moderators(path.clone().into(), context.reset_request_count()).await?;
     assert_eq!(200, res.status());
     let res =
-      get_apub_community_moderators(query.clone().into(), context.reset_request_count()).await?;
+      get_apub_community_outbox(path.into(), context.reset_request_count(), request).await?;
-    assert_eq!(200, res.status());
-    let res =
-      get_apub_community_outbox(query.into(), context.reset_request_count(), request).await?;
     assert_eq!(200, res.status());
 
     Instance::delete(&mut context.pool(), instance.id).await?;
@@ -227,28 +284,35 @@ pub(crate) mod tests {
     let request = TestRequest::default().to_http_request();
 
     // should return tombstone
-    let query = CommunityQuery {
+    let path: Path<CommunityPath> = CommunityPath {
       community_name: community.name.clone(),
-    };
+    }
-    let res = get_apub_community_http(query.clone().into(), context.reset_request_count()).await?;
+    .into();
+    let res = get_apub_community_http(path.clone().into(), context.reset_request_count()).await?;
     assert_eq!(410, res.status());
     let res_tombstone = decode_response::<Tombstone>(res).await;
     assert!(res_tombstone.is_ok());
 
     let res = get_apub_community_featured(
-      query.clone().into(),
+      path.clone().into(),
+      context.reset_request_count(),
+      request.clone(),
+    )
+    .await;
+    assert!(res.is_err());
+    let query = Query(CommunityIsFollowerQuery { is_follower: None });
+    let res = get_apub_community_followers(
+      path.clone().into(),
+      query,
       context.reset_request_count(),
       request.clone(),
     )
     .await;
     assert!(res.is_err());
     let res =
-      get_apub_community_followers(query.clone().into(), context.reset_request_count()).await;
+      get_apub_community_moderators(path.clone().into(), context.reset_request_count()).await;
     assert!(res.is_err());
-    let res =
+    let res = get_apub_community_outbox(path, context.reset_request_count(), request).await;
-      get_apub_community_moderators(query.clone().into(), context.reset_request_count()).await;
-    assert!(res.is_err());
-    let res = get_apub_community_outbox(query.into(), context.reset_request_count(), request).await;
     assert!(res.is_err());
 
     //Community::delete(&mut context.pool(), community.id).await?;
@@ -263,25 +327,32 @@ pub(crate) mod tests {
     let (instance, community) = init(false, CommunityVisibility::LocalOnly, &context).await?;
     let request = TestRequest::default().to_http_request();
 
-    let query = CommunityQuery {
+    let path: Path<CommunityPath> = CommunityPath {
       community_name: community.name.clone(),
-    };
+    }
-    let res = get_apub_community_http(query.clone().into(), context.reset_request_count()).await;
+    .into();
+    let res = get_apub_community_http(path.clone().into(), context.reset_request_count()).await;
     assert!(res.is_err());
     let res = get_apub_community_featured(
-      query.clone().into(),
+      path.clone().into(),
+      context.reset_request_count(),
+      request.clone(),
+    )
+    .await;
+    assert!(res.is_err());
+    let query = Query(CommunityIsFollowerQuery { is_follower: None });
+    let res = get_apub_community_followers(
+      path.clone().into(),
+      query,
       context.reset_request_count(),
       request.clone(),
     )
     .await;
     assert!(res.is_err());
     let res =
-      get_apub_community_followers(query.clone().into(), context.reset_request_count()).await;
+      get_apub_community_moderators(path.clone().into(), context.reset_request_count()).await;
     assert!(res.is_err());
-    let res =
+    let res = get_apub_community_outbox(path, context.reset_request_count(), request).await;
-      get_apub_community_moderators(query.clone().into(), context.reset_request_count()).await;
-    assert!(res.is_err());
-    let res = get_apub_community_outbox(query.into(), context.reset_request_count(), request).await;
     assert!(res.is_err());
 
     Instance::delete(&mut context.pool(), instance.id).await?;

crates/apub/src/http/mod.rs

@@ -8,6 +8,7 @@ use activitypub_federation::{
   actix_web::{inbox::receive_activity, signing_actor},
   config::Data,
   protocol::context::WithContext,
+  traits::Actor,
   FEDERATION_CONTENT_TYPE,
 };
 use actix_web::{web, web::Bytes, HttpRequest, HttpResponse};
@@ -145,6 +146,7 @@ async fn check_community_content_fetchable(
     // from the fetching instance then fetching is allowed
     Private => {
       let signing_actor = signing_actor::<SiteOrCommunityOrUser>(request, None, context).await?;
+      if community.local {
         Ok(
           CommunityFollowerView::check_has_followers_from_instance(
             community.id,
@@ -153,6 +155,18 @@ async fn check_community_content_fetchable(
           )
           .await?,
         )
+      } else if let Some(followers_url) = community.followers_url.clone() {
+        let mut followers_url = followers_url.inner().clone();
+        followers_url
+          .query_pairs_mut()
+          .append_pair("is_follower", signing_actor.id().as_str());
+        let req = context.client().get(followers_url.as_str());
+        let req = context.sign_request(req, Bytes::new()).await?;
+        context.client().execute(req).await?.error_for_status()?;
+        Ok(())
+      } else {
+        Err(LemmyErrorType::NotFound.into())
+      }
     }
   }
 }

crates/db_views_actor/src/community_follower_view.rs

@@ -232,6 +232,25 @@ impl CommunityFollowerView {
     .then_some(())
     .ok_or(diesel::NotFound)
   }
+
+  pub async fn is_follower(
+    community_id: CommunityId,
+    instance_id: InstanceId,
+    pool: &mut DbPool<'_>,
+  ) -> Result<(), Error> {
+    let conn = &mut get_conn(pool).await?;
+    select(exists(
+      action_query(community_actions::followed)
+        .inner_join(person::table.on(community_actions::person_id.eq(person::id)))
+        .filter(community_actions::community_id.eq(community_id))
+        .filter(person::instance_id.eq(instance_id))
+        .filter(community_actions::follow_state.eq(CommunityFollowerState::Accepted)),
+    ))
+    .get_result::<bool>(conn)
+    .await?
+    .then_some(())
+    .ok_or(diesel::NotFound)
+  }
 }
 
 #[cfg(test)]