Fix check for federated mod actions (#2489)

pull/2487/head
Nutomic 2022-10-10 15:20:36 +00:00 committed by GitHub
parent d2608bb279
commit ec5e63b5a9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 64 additions and 39 deletions

View File

@ -4,6 +4,7 @@ import { CommunityView } from 'lemmy-js-client';
import { import {
alpha, alpha,
beta, beta,
gamma,
setupLogins, setupLogins,
resolveCommunity, resolveCommunity,
createCommunity, createCommunity,
@ -11,6 +12,12 @@ import {
removeCommunity, removeCommunity,
getCommunity, getCommunity,
followCommunity, followCommunity,
banPersonFromCommunity,
resolvePerson,
getSite,
createPost,
getPost,
resolvePost,
} from './shared'; } from './shared';
beforeAll(async () => { beforeAll(async () => {
@ -162,3 +169,33 @@ test('Search for beta community', async () => {
let alphaCommunity = (await resolveCommunity(alpha, searchShort)).community.unwrap(); let alphaCommunity = (await resolveCommunity(alpha, searchShort)).community.unwrap();
assertCommunityFederation(alphaCommunity, communityRes.community_view); assertCommunityFederation(alphaCommunity, communityRes.community_view);
}); });
test('Admin actions in remote community are not federated to origin', async () => {
// create a community on alpha
let communityRes = (await createCommunity(alpha)).community_view;
expect(communityRes.community.name).toBeDefined();
// gamma follows community and posts in it
let gammaCommunity = (await resolveCommunity(gamma, communityRes.community.actor_id)).community.unwrap();
let gammaFollow = (await followCommunity(gamma, true, gammaCommunity.community.id));
expect(gammaFollow.community_view.subscribed).toBe("Subscribed");
let gammaPost = (await createPost(gamma, gammaCommunity.community.id)).post_view;
expect(gammaPost.post.id).toBeDefined();
expect(gammaPost.creator_banned_from_community).toBe(false);
// admin of beta decides to ban gamma from community
let betaCommunity = (await resolveCommunity(beta, communityRes.community.actor_id)).community.unwrap();
let bannedUserInfo1 = (await getSite(gamma)).my_user.unwrap().local_user_view.person;
let bannedUserInfo2 = (await resolvePerson(beta, bannedUserInfo1.actor_id)).person.unwrap();
let banRes = (await banPersonFromCommunity(beta, bannedUserInfo2.person.id, betaCommunity.community.id, true, true));
console.log(banRes);
expect(banRes.banned).toBe(true);
// ban doesnt federate to community's origin instance alpha
let alphaPost = (await resolvePost(alpha, gammaPost.post)).post.unwrap();
expect(alphaPost.creator_banned_from_community).toBe(false);
// and neither to gamma
let gammaPost2 = (await getPost(gamma, gammaPost.post.id));
expect(gammaPost2.post_view.creator_banned_from_community).toBe(false);
});

View File

@ -145,7 +145,7 @@ impl ActivityHandler for BlockUser {
verify_mod_action( verify_mod_action(
&self.actor, &self.actor,
self.object.inner(), self.object.inner(),
&community, community.id,
context, context,
request_counter, request_counter,
) )

View File

@ -90,7 +90,7 @@ impl ActivityHandler for AddMod {
verify_mod_action( verify_mod_action(
&self.actor, &self.actor,
self.object.inner(), self.object.inner(),
&community, community.id,
context, context,
request_counter, request_counter,
) )

View File

@ -90,7 +90,7 @@ impl ActivityHandler for RemoveMod {
verify_mod_action( verify_mod_action(
&self.actor, &self.actor,
self.object.inner(), self.object.inner(),
&community, community.id,
context, context,
request_counter, request_counter,
) )

View File

@ -78,7 +78,7 @@ impl ActivityHandler for UpdateCommunity {
verify_mod_action( verify_mod_action(
&self.actor, &self.actor,
self.object.id.inner(), self.object.id.inner(),
&community, community.id,
context, context,
request_counter, request_counter,
) )

View File

@ -120,7 +120,7 @@ impl ActivityHandler for CreateOrUpdatePost {
verify_mod_action( verify_mod_action(
&self.actor, &self.actor,
self.object.id.inner(), self.object.id.inner(),
&community, community.id,
context, context,
request_counter, request_counter,
) )

View File

@ -159,7 +159,7 @@ pub(in crate::activities) async fn verify_delete_activity(
verify_mod_action( verify_mod_action(
&activity.actor, &activity.actor,
activity.object.id(), activity.object.id(),
&community, community.id,
context, context,
request_counter, request_counter,
) )
@ -208,7 +208,7 @@ async fn verify_delete_post_or_comment(
) -> Result<(), LemmyError> { ) -> Result<(), LemmyError> {
verify_person_in_community(actor, community, context, request_counter).await?; verify_person_in_community(actor, community, context, request_counter).await?;
if is_mod_action { if is_mod_action {
verify_mod_action(actor, object_id, community, context, request_counter).await?; verify_mod_action(actor, object_id, community.id, context, request_counter).await?;
} else { } else {
// domain of post ap_id and post.creator ap_id are identical, so we just check the former // domain of post ap_id and post.creator ap_id are identical, so we just check the former
verify_domains_match(actor.inner(), object_id)?; verify_domains_match(actor.inner(), object_id)?;

View File

@ -14,7 +14,7 @@ use activitypub_federation::{
use activitystreams_kinds::public; use activitystreams_kinds::public;
use anyhow::anyhow; use anyhow::anyhow;
use lemmy_api_common::utils::blocking; use lemmy_api_common::utils::blocking;
use lemmy_db_schema::source::community::Community; use lemmy_db_schema::{newtypes::CommunityId, source::community::Community};
use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView}; use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView};
use lemmy_utils::error::LemmyError; use lemmy_utils::error::LemmyError;
use lemmy_websocket::LemmyContext; use lemmy_websocket::LemmyContext;
@ -75,9 +75,7 @@ pub(crate) async fn verify_person_in_community(
Ok(()) Ok(())
} }
/// Verify that the actor is a community mod. This check is only run if the community is local, /// Verify that mod action in community was performed by a moderator.
/// because in case of remote communities, admins can also perform mod actions. As admin status
/// is not federated, we cant verify their actions remotely.
/// ///
/// * `mod_id` - Activitypub ID of the mod or admin who performed the action /// * `mod_id` - Activitypub ID of the mod or admin who performed the action
/// * `object_id` - Activitypub ID of the actor or object that is being moderated /// * `object_id` - Activitypub ID of the actor or object that is being moderated
@ -86,40 +84,30 @@ pub(crate) async fn verify_person_in_community(
pub(crate) async fn verify_mod_action( pub(crate) async fn verify_mod_action(
mod_id: &ObjectId<ApubPerson>, mod_id: &ObjectId<ApubPerson>,
object_id: &Url, object_id: &Url,
community: &ApubCommunity, community_id: CommunityId,
context: &LemmyContext, context: &LemmyContext,
request_counter: &mut i32, request_counter: &mut i32,
) -> Result<(), LemmyError> { ) -> Result<(), LemmyError> {
if community.local { let mod_ = mod_id
let actor = mod_id
.dereference(context, local_instance(context), request_counter) .dereference(context, local_instance(context), request_counter)
.await?; .await?;
// Note: this will also return true for admins in addition to mods, but as we dont know about
// remote admins, it doesnt make any difference.
let community_id = community.id;
let actor_id = actor.id;
let is_mod_or_admin = blocking(context.pool(), move |conn| { let is_mod_or_admin = blocking(context.pool(), move |conn| {
CommunityView::is_mod_or_admin(conn, actor_id, community_id) CommunityView::is_mod_or_admin(conn, mod_.id, community_id)
}) })
.await?; .await?;
// mod action was done either by a community mod or a local admin, so its allowed
if is_mod_or_admin { if is_mod_or_admin {
return Ok(()); return Ok(());
} }
// mod action comes from the same instance as the moderated object, so it was presumably done // mod action comes from the same instance as the moderated object, so it was presumably done
// by an instance admin and is legitimate (admin status is not federated). // by an instance admin.
// TODO: federate instance admin status and check it here
if mod_id.inner().domain() == object_id.domain() { if mod_id.inner().domain() == object_id.domain() {
return Ok(()); return Ok(());
} }
// the user is not a valid mod Err(LemmyError::from_message("Not a mod"))
return Err(LemmyError::from_message("Not a mod"));
}
Ok(())
} }
/// For Add/Remove community moderator activities, check that the target field actually contains /// For Add/Remove community moderator activities, check that the target field actually contains