Ignore activities in remote communities without local followers (#4006)

* Ignore activities in remote communities without local followers (fixes #3568)

* x

* comments

* prettier

* fix api test

* fix test

* cleanup

* fix remaining test

* clippy

* decrease delay
This commit is contained in:
Nutomic 2023-10-16 12:03:49 +02:00 committed by GitHub
parent 3be56ef2e0
commit 256ee61908
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 917 additions and 809 deletions

View file

@ -35,6 +35,7 @@ import {
delay, delay,
waitForPost, waitForPost,
alphaUrl, alphaUrl,
followCommunity,
} from "./shared"; } from "./shared";
import { CommentView } from "lemmy-js-client/dist/types/CommentView"; import { CommentView } from "lemmy-js-client/dist/types/CommentView";
import { CommunityView } from "lemmy-js-client"; import { CommunityView } from "lemmy-js-client";
@ -500,6 +501,13 @@ test("A and G subscribe to B (center) A posts, G mentions B, it gets announced t
throw "Missing alpha community"; throw "Missing alpha community";
} }
// follow community from beta so that it accepts the mention
let betaCommunity = await resolveCommunity(
beta,
alphaCommunity.community.actor_id,
);
await followCommunity(beta, true, betaCommunity.community!.community.id);
let alphaPost = await createPost(alpha, alphaCommunity.community.id); let alphaPost = await createPost(alpha, alphaCommunity.community.id);
expect(alphaPost.post_view.community.local).toBe(true); expect(alphaPost.post_view.community.local).toBe(true);

View file

@ -28,8 +28,10 @@ import {
delay, delay,
waitForPost, waitForPost,
alphaUrl, alphaUrl,
betaAllowedInstances,
searchPostLocal,
} from "./shared"; } from "./shared";
import { LemmyHttp } from "lemmy-js-client"; import { EditSite, LemmyHttp } from "lemmy-js-client";
beforeAll(async () => { beforeAll(async () => {
await setupLogins(); await setupLogins();
@ -376,3 +378,56 @@ test("User blocks instance, communities are hidden", async () => {
let listing_ids3 = listing3.posts.map(p => p.post.ap_id); let listing_ids3 = listing3.posts.map(p => p.post.ap_id);
expect(listing_ids3).toContain(postRes.post_view.post.ap_id); expect(listing_ids3).toContain(postRes.post_view.post.ap_id);
}); });
test("Dont receive community activities after unsubscribe", async () => {
let communityRes = await createCommunity(alpha);
expect(communityRes.community_view.community.name).toBeDefined();
expect(communityRes.community_view.counts.subscribers).toBe(1);
let betaCommunity = (
await resolveCommunity(beta, communityRes.community_view.community.actor_id)
).community;
assertCommunityFederation(betaCommunity, communityRes.community_view);
// follow alpha community from beta
await followCommunity(beta, true, betaCommunity!.community.id);
// ensure that follower count was updated
let communityRes1 = await getCommunity(
alpha,
communityRes.community_view.community.id,
);
expect(communityRes1.community_view.counts.subscribers).toBe(2);
// temporarily block alpha, so that it doesnt know about unfollow
let editSiteForm: EditSite = {};
editSiteForm.allowed_instances = ["lemmy-epsilon"];
await beta.editSite(editSiteForm);
await delay(2000);
// unfollow
await followCommunity(beta, false, betaCommunity!.community.id);
// ensure that alpha still sees beta as follower
let communityRes2 = await getCommunity(
alpha,
communityRes.community_view.community.id,
);
expect(communityRes2.community_view.counts.subscribers).toBe(2);
// unblock alpha
editSiteForm.allowed_instances = betaAllowedInstances;
await beta.editSite(editSiteForm);
await delay(2000);
// create a post, it shouldnt reach beta
let postRes = await createPost(
alpha,
communityRes.community_view.community.id,
);
expect(postRes.post_view.post.id).toBeDefined();
await delay(2000);
let postResBeta = searchPostLocal(beta, postRes.post_view.post);
expect((await postResBeta).posts.length).toBe(0);
});

View file

@ -84,6 +84,13 @@ export let gamma = new LemmyHttp(gammaUrl);
export let delta = new LemmyHttp(deltaUrl); export let delta = new LemmyHttp(deltaUrl);
export let epsilon = new LemmyHttp(epsilonUrl); export let epsilon = new LemmyHttp(epsilonUrl);
export let betaAllowedInstances = [
"lemmy-alpha",
"lemmy-gamma",
"lemmy-delta",
"lemmy-epsilon",
];
const password = "lemmylemmy"; const password = "lemmylemmy";
export async function setupLogins() { export async function setupLogins() {
@ -150,12 +157,7 @@ export async function setupLogins() {
]; ];
await alpha.editSite(editSiteForm); await alpha.editSite(editSiteForm);
editSiteForm.allowed_instances = [ editSiteForm.allowed_instances = betaAllowedInstances;
"lemmy-alpha",
"lemmy-gamma",
"lemmy-delta",
"lemmy-epsilon",
];
await beta.editSite(editSiteForm); await beta.editSite(editSiteForm);
editSiteForm.allowed_instances = [ editSiteForm.allowed_instances = [

File diff suppressed because it is too large Load diff

View file

@ -22,8 +22,8 @@ use activitypub_federation::{
traits::{ActivityHandler, Actor}, traits::{ActivityHandler, Actor},
}; };
use lemmy_api_common::context::LemmyContext; use lemmy_api_common::context::LemmyContext;
use lemmy_db_schema::source::activity::ActivitySendTargets; use lemmy_db_schema::source::{activity::ActivitySendTargets, community::CommunityFollower};
use lemmy_utils::error::{LemmyError, LemmyErrorType}; use lemmy_utils::error::{LemmyError, LemmyErrorType, LemmyResult};
use serde_json::Value; use serde_json::Value;
use url::Url; use url::Url;
@ -46,24 +46,28 @@ impl ActivityHandler for RawAnnouncableActivities {
} }
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
async fn receive(self, data: &Data<Self::DataType>) -> Result<(), Self::Error> { async fn receive(self, context: &Data<Self::DataType>) -> Result<(), Self::Error> {
let activity: AnnouncableActivities = self.clone().try_into()?; let activity: AnnouncableActivities = self.clone().try_into()?;
// This is only for sending, not receiving so we reject it. // This is only for sending, not receiving so we reject it.
if let AnnouncableActivities::Page(_) = activity { if let AnnouncableActivities::Page(_) = activity {
Err(LemmyErrorType::CannotReceivePage)? Err(LemmyErrorType::CannotReceivePage)?
} }
// verify and receive activity // Need to treat community as optional here because `Delete/PrivateMessage` gets routed through
activity.verify(data).await?; let community = activity.community(context).await.ok();
activity.clone().receive(data).await?; can_accept_activity_in_community(&community, context).await?;
// if activity is in a community, send to followers // verify and receive activity
let community = activity.community(data).await; activity.verify(context).await?;
if let Ok(community) = community { activity.clone().receive(context).await?;
// if community is local, send activity to followers
if let Some(community) = community {
if community.local { if community.local {
let actor_id = activity.actor().clone().into(); let actor_id = activity.actor().clone().into();
verify_person_in_community(&actor_id, &community, data).await?; verify_person_in_community(&actor_id, &community, context).await?;
AnnounceActivity::send(self, &community, data).await?; AnnounceActivity::send(self, &community, context).await?;
} }
} }
Ok(()) Ok(())
@ -150,11 +154,15 @@ impl ActivityHandler for AnnounceActivity {
#[tracing::instrument(skip_all)] #[tracing::instrument(skip_all)]
async fn receive(self, context: &Data<Self::DataType>) -> Result<(), LemmyError> { async fn receive(self, context: &Data<Self::DataType>) -> Result<(), LemmyError> {
let object: AnnouncableActivities = self.object.object(context).await?.try_into()?; let object: AnnouncableActivities = self.object.object(context).await?.try_into()?;
// This is only for sending, not receiving so we reject it. // This is only for sending, not receiving so we reject it.
if let AnnouncableActivities::Page(_) = object { if let AnnouncableActivities::Page(_) = object {
Err(LemmyErrorType::CannotReceivePage)? Err(LemmyErrorType::CannotReceivePage)?
} }
let community = object.community(context).await?;
can_accept_activity_in_community(&Some(community), context).await?;
// verify here in order to avoid fetching the object twice over http // verify here in order to avoid fetching the object twice over http
object.verify(context).await?; object.verify(context).await?;
object.receive(context).await object.receive(context).await
@ -185,3 +193,23 @@ impl TryFrom<AnnouncableActivities> for RawAnnouncableActivities {
serde_json::from_value(serde_json::to_value(value)?) serde_json::from_value(serde_json::to_value(value)?)
} }
} }
/// Check if an activity in the given community can be accepted. To return true, the community must
/// either be local to this instance, or it must have at least one local follower.
///
/// TODO: This means mentions dont work if the community has no local followers. Can be fixed
/// by checking if any local user is in to/cc fields of activity. Anyway this is a minor
/// problem compared to receiving unsolicited posts.
async fn can_accept_activity_in_community(
community: &Option<ApubCommunity>,
context: &Data<LemmyContext>,
) -> LemmyResult<()> {
if let Some(community) = community {
if !community.local
&& !CommunityFollower::has_local_followers(&mut context.pool(), community.id).await?
{
Err(LemmyErrorType::CommunityHasNoFollowers)?
}
}
Ok(())
}

View file

@ -20,7 +20,6 @@ use lemmy_db_schema::{
post::{PostSaved, PostSavedForm}, post::{PostSaved, PostSavedForm},
}, },
traits::{Blockable, Crud, Followable, Saveable}, traits::{Blockable, Crud, Followable, Saveable},
utils::diesel_option_overwrite,
}; };
use lemmy_db_views::structs::LocalUserView; use lemmy_db_views::structs::LocalUserView;
use lemmy_utils::{ use lemmy_utils::{
@ -97,12 +96,9 @@ pub async fn import_settings(
local_user_view: LocalUserView, local_user_view: LocalUserView,
context: Data<LemmyContext>, context: Data<LemmyContext>,
) -> Result<Json<SuccessResponse>, LemmyError> { ) -> Result<Json<SuccessResponse>, LemmyError> {
let display_name = diesel_option_overwrite(data.display_name.clone());
let bio = diesel_option_overwrite(data.bio.clone());
let person_form = PersonUpdateForm { let person_form = PersonUpdateForm {
display_name, display_name: Some(data.display_name.clone()),
bio, bio: Some(data.bio.clone()),
matrix_user_id: Some(data.matrix_id.clone()), matrix_user_id: Some(data.matrix_id.clone()),
bot_account: data.bot_account, bot_account: data.bot_account,
..Default::default() ..Default::default()

View file

@ -229,6 +229,22 @@ impl CommunityFollower {
pub fn select_subscribed_type() -> dsl::Nullable<community_follower::pending> { pub fn select_subscribed_type() -> dsl::Nullable<community_follower::pending> {
community_follower::pending.nullable() community_follower::pending.nullable()
} }
/// Check if a remote instance has any followers on local instance. For this it is enough to check
/// if any follow relation is stored. Dont use this for local community.
pub async fn has_local_followers(
pool: &mut DbPool<'_>,
remote_community_id: CommunityId,
) -> Result<bool, Error> {
use crate::schema::community_follower::dsl::{community_follower, community_id};
use diesel::dsl::{exists, select};
let conn = &mut get_conn(pool).await?;
select(exists(
community_follower.filter(community_id.eq(remote_community_id)),
))
.get_result(conn)
.await
}
} }
impl Queryable<sql_types::Nullable<sql_types::Bool>, Pg> for SubscribedType { impl Queryable<sql_types::Nullable<sql_types::Bool>, Pg> for SubscribedType {

View file

@ -215,6 +215,7 @@ pub enum LemmyErrorType {
InstanceBlockAlreadyExists, InstanceBlockAlreadyExists,
/// `jwt` cookie must be marked secure and httponly /// `jwt` cookie must be marked secure and httponly
AuthCookieInsecure, AuthCookieInsecure,
CommunityHasNoFollowers,
UserBackupTooLarge, UserBackupTooLarge,
Unknown(String), Unknown(String),
} }