From b8dda06f5b80efeb06eb2b39d747daafbbbbbefd Mon Sep 17 00:00:00 2001 From: Nutomic Date: Fri, 22 Nov 2024 15:49:56 +0000 Subject: [PATCH] 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 | 4 +- Cargo.toml | 2 +- api_tests/src/private_community.spec.ts | 165 ++++++++++++++++-- crates/api_common/src/build_response.rs | 35 ++-- .../apub/src/activities/community/announce.rs | 2 +- crates/apub/src/activities/community/mod.rs | 2 +- .../activities/create_or_update/comment.rs | 3 + crates/apub/src/http/community.rs | 141 +++++++++++---- crates/apub/src/http/mod.rs | 28 ++- .../src/community_follower_view.rs | 19 ++ 10 files changed, 332 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index edc6722bf..c1b1f3673 100644 --- a/Cargo.lock +++ b/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", diff --git a/Cargo.toml b/Cargo.toml index d9c868ee7..bce88af67 100644 --- a/Cargo.toml +++ b/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" diff --git a/api_tests/src/private_community.spec.ts b/api_tests/src/private_community.spec.ts index 76faf800f..65340a1dd 100644 --- a/api_tests/src/private_community.spec.ts +++ b/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( - () => listCommunityPendingFollows(alpha), - f => f.items.length == 1, - ); - const approve = await approveCommunityPendingFollow( - alpha, - alphaCommunityId, - pendingFollows1.items[0].person.id, - ); - expect(approve.success).toBe(true); + approveFollower(alpha, alphaCommunityId); // 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); +} diff --git a/crates/api_common/src/build_response.rs b/crates/api_common/src/build_response.rs index d40f4c23d..b73c0e482 100644 --- a/crates/api_common/src/build_response.rs +++ b/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; - // Read the comment view to get extra info - let comment_view = CommentView::read( - &mut context.pool(), - comment_id, - local_user_view.map(|view| &view.local_user), - ) - .await?; - let comment = comment_view.comment; - let post = comment_view.post; - let community = comment_view.community; + // When called from api code, we have local user view and can read with CommentView + // 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, + Some(&local_user_view.local_user), + ) + .await?; + ( + comment_view.comment, + 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 diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index b31e4b74f..950f4861d 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/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 { diff --git a/crates/apub/src/activities/community/mod.rs b/crates/apub/src/activities/community/mod.rs index 59b8fadbb..93c6e5c77 100644 --- a/crates/apub/src/activities/community/mod.rs +++ b/crates/apub/src/activities/community/mod.rs @@ -42,7 +42,7 @@ pub(crate) async fn send_activity_in_community( context: &Data, ) -> LemmyResult<()> { // If community is local only, don't send anything out - if community.visibility != CommunityVisibility::Public { + if community.visibility == CommunityVisibility::LocalOnly { return Ok(()); } diff --git a/crates/apub/src/activities/create_or_update/comment.rs b/crates/apub/src/activities/create_or_update/comment.rs index 90ab0153f..9f64e805b 100644 --- a/crates/apub/src/activities/create_or_update/comment.rs +++ b/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(()) } diff --git a/crates/apub/src/http/community.rs b/crates/apub/src/http/community.rs index 96a917d91..dbcc51258 100644 --- a/crates/apub/src/http/community.rs +++ b/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>, +} + /// 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, + info: Path, context: Data, ) -> LemmyResult { 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, + info: Path, + query: Query, context: Data, + request: HttpRequest, ) -> LemmyResult { 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, + context: Data, + request: HttpRequest, +) -> LemmyResult { + 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::(&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, + info: Path, context: Data, request: HttpRequest, ) -> LemmyResult { @@ -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, + info: Path, context: Data, ) -> LemmyResult { 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, + info: Path, context: Data, request: HttpRequest, ) -> LemmyResult { @@ -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?; - assert_eq!(200, res.status()); - let res = - get_apub_community_outbox(query.into(), context.reset_request_count(), request).await?; + get_apub_community_outbox(path.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 { 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::(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 = - 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; + let res = get_apub_community_outbox(path, 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 { 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 = - 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; + let res = get_apub_community_outbox(path, context.reset_request_count(), request).await; assert!(res.is_err()); Instance::delete(&mut context.pool(), instance.id).await?; diff --git a/crates/apub/src/http/mod.rs b/crates/apub/src/http/mod.rs index 8780f8789..fc2fbf0d3 100644 --- a/crates/apub/src/http/mod.rs +++ b/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,14 +146,27 @@ async fn check_community_content_fetchable( // from the fetching instance then fetching is allowed Private => { let signing_actor = signing_actor::(request, None, context).await?; - Ok( - CommunityFollowerView::check_has_followers_from_instance( - community.id, - signing_actor.instance_id(), - &mut context.pool(), + if community.local { + Ok( + CommunityFollowerView::check_has_followers_from_instance( + community.id, + signing_actor.instance_id(), + &mut context.pool(), + ) + .await?, ) - .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()) + } } } } diff --git a/crates/db_views_actor/src/community_follower_view.rs b/crates/db_views_actor/src/community_follower_view.rs index 56f4cca93..c32ccb5b8 100644 --- a/crates/db_views_actor/src/community_follower_view.rs +++ b/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::(conn) + .await? + .then_some(()) + .ok_or(diesel::NotFound) + } } #[cfg(test)]