Skip to content

Commit

Permalink
Fix federation of admin actions (fixes #3980) (#3988)
Browse files Browse the repository at this point in the history
* Fix federation of admin actions (fixes #3980)

* clippy

---------

Co-authored-by: Dessalines <tyhou13@gmx.com>
  • Loading branch information
Nutomic and dessalines committed Sep 26, 2023
1 parent 9a9ece8 commit 5058911
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 30 deletions.
13 changes: 10 additions & 3 deletions api_tests/src/post.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,22 +329,29 @@ test("Remove a post from admin and community on same instance", async () => {
throw "Missing beta community";
}
await followBeta(alpha);
let postRes = await createPost(alpha, betaCommunity.community.id);
let gammaCommunity = await resolveCommunity(
gamma,
betaCommunity.community.actor_id,
);
let postRes = await createPost(gamma, gammaCommunity.community!.community.id);
expect(postRes.post_view.post).toBeDefined();
// Get the id for beta
let betaPost = await waitForPost(beta, postRes.post_view.post);
expect(betaPost).toBeDefined();

let alphaPost0 = await waitForPost(alpha, postRes.post_view.post);
expect(alphaPost0).toBeDefined();

// The beta admin removes it (the community lives on beta)
let removePostRes = await removePost(beta, true, betaPost.post);
expect(removePostRes.post_view.post.removed).toBe(true);

// Make sure lemmy alpha sees post is removed
let alphaPost = await waitUntil(
() => getPost(alpha, postRes.post_view.post.id),
() => getPost(alpha, alphaPost0.post.id),
p => p?.post_view.post.removed ?? false,
);
expect(alphaPost.post_view?.post.removed).toBe(true);
expect(alphaPost?.post_view.post.removed).toBe(true);
assertPostFederation(alphaPost.post_view, removePostRes.post_view);

// Undelete
Expand Down
2 changes: 1 addition & 1 deletion crates/apub/src/activities/block/block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl ActivityHandler for BlockUser {
}
SiteOrCommunity::Community(community) => {
verify_person_in_community(&self.actor, &community, context).await?;
verify_mod_action(&self.actor, self.object.inner(), community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
}
}
Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/apub/src/activities/community/collection_add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl ActivityHandler for CollectionAdd {
verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?;
verify_mod_action(&self.actor, &self.object, community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/apub/src/activities/community/collection_remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ impl ActivityHandler for CollectionRemove {
verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?;
verify_mod_action(&self.actor, &self.object, community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
Ok(())
}

Expand Down
10 changes: 2 additions & 8 deletions crates/apub/src/activities/community/lock_page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ActivityHandler for LockPage {
let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?;
check_community_deleted_or_removed(&community)?;
verify_mod_action(&self.actor, self.object.inner(), community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
Ok(())
}

Expand Down Expand Up @@ -86,13 +86,7 @@ impl ActivityHandler for UndoLockPage {
let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?;
check_community_deleted_or_removed(&community)?;
verify_mod_action(
&self.actor,
self.object.object.inner(),
community.id,
context,
)
.await?;
verify_mod_action(&self.actor, &community, context).await?;
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/apub/src/activities/community/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl ActivityHandler for UpdateCommunity {
verify_is_public(&self.to, &self.cc)?;
let community = self.community(context).await?;
verify_person_in_community(&self.actor, &community, context).await?;
verify_mod_action(&self.actor, self.object.id.inner(), community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
ApubCommunity::verify(&self.object, &community.actor_id.clone().into(), context).await?;
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion crates/apub/src/activities/create_or_update/post.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ impl ActivityHandler for CreateOrUpdatePage {
CreateOrUpdateType::Update => {
let is_mod_action = self.object.is_mod_action(context).await?;
if is_mod_action {
verify_mod_action(&self.actor, self.object.id.inner(), community.id, context).await?;
verify_mod_action(&self.actor, &community, context).await?;
} else {
verify_domains_match(self.actor.inner(), self.object.id.inner())?;
verify_urls_match(self.actor.inner(), self.object.creator()?.inner())?;
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/activities/deletion/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ pub(in crate::activities) async fn verify_delete_activity(
verify_person_in_community(&activity.actor, &community, context).await?;
}
// community deletion is always a mod (or admin) action
verify_mod_action(&activity.actor, activity.object.id(), community.id, context).await?;
verify_mod_action(&activity.actor, &community, context).await?;
}
DeletableObjects::Post(p) => {
verify_is_public(&activity.to, &[])?;
Expand Down Expand Up @@ -231,7 +231,7 @@ async fn verify_delete_post_or_comment(
) -> Result<(), LemmyError> {
verify_person_in_community(actor, community, context).await?;
if is_mod_action {
verify_mod_action(actor, object_id, community.id, context).await?;
verify_mod_action(actor, community, context).await?;
} else {
// 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)?;
Expand Down
18 changes: 7 additions & 11 deletions crates/apub/src/activities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,9 @@ use lemmy_api_common::{
context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData},
};
use lemmy_db_schema::{
newtypes::CommunityId,
source::{
activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm},
community::Community,
},
use lemmy_db_schema::source::{
activity::{ActivitySendTargets, ActorType, SentActivity, SentActivityForm},
community::Community,
};
use lemmy_db_views_actor::structs::{CommunityPersonBanView, CommunityView};
use lemmy_utils::error::{LemmyError, LemmyErrorExt, LemmyErrorType, LemmyResult};
Expand Down Expand Up @@ -113,22 +110,21 @@ pub(crate) async fn verify_person_in_community(
#[tracing::instrument(skip_all)]
pub(crate) async fn verify_mod_action(
mod_id: &ObjectId<ApubPerson>,
object_id: &Url,
community_id: CommunityId,
community: &Community,
context: &Data<LemmyContext>,
) -> Result<(), LemmyError> {
let mod_ = mod_id.dereference(context).await?;

let is_mod_or_admin =
CommunityView::is_mod_or_admin(&mut context.pool(), mod_.id, community_id).await?;
CommunityView::is_mod_or_admin(&mut context.pool(), mod_.id, community.id).await?;
if is_mod_or_admin {
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 community, so it was presumably done
// 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() == community.actor_id.domain() {
return Ok(());
}

Expand Down

0 comments on commit 5058911

Please sign in to comment.