Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding ability to restore content on user unban. #4845

Merged
merged 8 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api_tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"eslint": "^9.8.0",
"eslint-plugin-prettier": "^5.1.3",
"jest": "^29.5.0",
"lemmy-js-client": "0.19.5-alpha.1",
"lemmy-js-client": "0.20.0-alpha.4",
"prettier": "^3.2.5",
"ts-jest": "^29.1.0",
"typescript": "^5.5.4",
Expand Down
263 changes: 132 additions & 131 deletions api_tests/pnpm-lock.yaml

Large diffs are not rendered by default.

9 changes: 8 additions & 1 deletion api_tests/src/post.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,10 +502,17 @@ test("Enforce site ban federation for local user", async () => {
alpha,
alphaPerson.person.id,
false,
false,
true,
);
expect(unBanAlpha.banned).toBe(false);

// existing alpha post should be restored on beta
betaBanRes = await waitUntil(
() => getPost(beta, searchBeta1.post.id),
s => !s.post_view.post.removed,
);
expect(betaBanRes.post_view.post.removed).toBe(false);

// Login gets invalidated by ban, need to login again
if (!alphaUserPerson) {
throw "Missing alpha person";
Expand Down
8 changes: 4 additions & 4 deletions api_tests/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,13 +419,13 @@ export async function banPersonFromSite(
api: LemmyHttp,
person_id: number,
ban: boolean,
remove_data: boolean,
remove_or_restore_data: boolean,
): Promise<BanPersonResponse> {
// Make sure lemmy-beta/c/main is cached on lemmy_alpha
let form: BanPerson = {
person_id,
ban,
remove_data,
remove_or_restore_data,
};
return api.banPerson(form);
}
Expand All @@ -434,13 +434,13 @@ export async function banPersonFromCommunity(
api: LemmyHttp,
person_id: number,
community_id: number,
remove_data: boolean,
remove_or_restore_data: boolean,
ban: boolean,
): Promise<BanFromCommunityResponse> {
let form: BanFromCommunity = {
person_id,
community_id,
remove_data: remove_data,
remove_or_restore_data,
ban,
};
return api.banFromCommunity(form);
Expand Down
20 changes: 15 additions & 5 deletions crates/api/src/community/ban.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@ use lemmy_api_common::{
community::{BanFromCommunity, BanFromCommunityResponse},
context::LemmyContext,
send_activity::{ActivityChannel, SendActivityData},
utils::{check_community_mod_action, check_expire_time, remove_user_data_in_community},
utils::{
check_community_mod_action,
check_expire_time,
remove_or_restore_user_data_in_community,
},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -33,7 +37,6 @@ pub async fn ban_from_community(
local_user_view: LocalUserView,
) -> LemmyResult<Json<BanFromCommunityResponse>> {
let banned_person_id = data.person_id;
let remove_data = data.remove_data.unwrap_or(false);
let expires = check_expire_time(data.expires)?;

// Verify that only mods or admins can ban
Expand Down Expand Up @@ -85,9 +88,16 @@ pub async fn ban_from_community(
}

// Remove/Restore their data if that's desired
if remove_data {
remove_user_data_in_community(data.community_id, banned_person_id, &mut context.pool()).await?;
}
if data.remove_or_restore_data.unwrap_or(false) {
let remove_data = data.ban;
remove_or_restore_user_data_in_community(
data.community_id,
banned_person_id,
remove_data,
&mut context.pool(),
)
.await?;
};

// Mod tables
let form = ModBanFromCommunityForm {
Expand Down
4 changes: 2 additions & 2 deletions crates/api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub(crate) async fn ban_nonlocal_user_from_local_communities(
target: &Person,
ban: bool,
reason: &Option<String>,
remove_data: &Option<bool>,
remove_or_restore_data: &Option<bool>,
expires: &Option<i64>,
context: &Data<LemmyContext>,
) -> LemmyResult<()> {
Expand Down Expand Up @@ -230,7 +230,7 @@ pub(crate) async fn ban_nonlocal_user_from_local_communities(
person_id: target.id,
ban,
reason: reason.clone(),
remove_data: *remove_data,
remove_or_restore_data: *remove_or_restore_data,
expires: *expires,
};

Expand Down
17 changes: 10 additions & 7 deletions crates/api/src/local_user/ban_person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use lemmy_api_common::{
context::LemmyContext,
person::{BanPerson, BanPersonResponse},
send_activity::{ActivityChannel, SendActivityData},
utils::{check_expire_time, is_admin, remove_user_data},
utils::{check_expire_time, is_admin, remove_user_data, restore_user_data},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -65,10 +65,13 @@ pub async fn ban_from_site(
}

// Remove their data if that's desired
let remove_data = data.remove_data.unwrap_or(false);
if remove_data {
remove_user_data(person.id, &context).await?;
}
if data.remove_or_restore_data.unwrap_or(false) {
if data.ban {
remove_user_data(person.id, &context).await?;
} else {
restore_user_data(person.id, &context).await?;
}
};

// Mod tables
let form = ModBanForm {
Expand All @@ -90,7 +93,7 @@ pub async fn ban_from_site(
&person,
data.ban,
&data.reason,
&data.remove_data,
&data.remove_or_restore_data,
&data.expires,
&context,
)
Expand All @@ -101,7 +104,7 @@ pub async fn ban_from_site(
moderator: local_user_view.person,
banned_user: person_view.person.clone(),
reason: data.reason.clone(),
remove_data: data.remove_data,
remove_or_restore_data: data.remove_or_restore_data,
ban: data.ban,
expires: data.expires,
},
Expand Down
2 changes: 1 addition & 1 deletion crates/api/src/site/purge/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ pub async fn purge_person(
moderator: local_user_view.person,
banned_user: person,
reason: data.reason.clone(),
remove_data: Some(true),
remove_or_restore_data: Some(true),
ban: true,
expires: None,
},
Expand Down
4 changes: 3 additions & 1 deletion crates/api_common/src/community.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ pub struct BanFromCommunity {
pub community_id: CommunityId,
pub person_id: PersonId,
pub ban: bool,
pub remove_data: Option<bool>,
/// Optionally remove or restore all their data. Useful for new troll accounts.
/// If ban is true, then this means remove. If ban is false, it means restore.
pub remove_or_restore_data: Option<bool>,
pub reason: Option<String>,
/// A time that the ban will expire, in unix epoch seconds.
///
Expand Down
5 changes: 3 additions & 2 deletions crates/api_common/src/person.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,9 @@ pub struct AddAdminResponse {
pub struct BanPerson {
pub person_id: PersonId,
pub ban: bool,
/// Optionally remove all their data. Useful for new troll accounts.
pub remove_data: Option<bool>,
/// Optionally remove or restore all their data. Useful for new troll accounts.
/// If ban is true, then this means remove. If ban is false, it means restore.
pub remove_or_restore_data: Option<bool>,
pub reason: Option<String>,
/// A time that the ban will expire, in unix epoch seconds.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/api_common/src/send_activity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ pub enum SendActivityData {
moderator: Person,
banned_user: Person,
reason: Option<String>,
remove_data: Option<bool>,
remove_or_restore_data: Option<bool>,
ban: bool,
expires: Option<i64>,
},
Expand Down
23 changes: 20 additions & 3 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -784,13 +784,30 @@ pub async fn remove_user_data(
Ok(())
}

pub async fn remove_user_data_in_community(
/// We can't restore their images, but we can unremove their posts and comments
pub async fn restore_user_data(
banned_person_id: PersonId,
context: &LemmyContext,
) -> LemmyResult<()> {
let pool = &mut context.pool();

// Posts
Post::update_removed_for_creator(pool, banned_person_id, None, false).await?;

// Comments
Comment::update_removed_for_creator(pool, banned_person_id, false).await?;

Ok(())
}

pub async fn remove_or_restore_user_data_in_community(
community_id: CommunityId,
banned_person_id: PersonId,
remove: bool,
pool: &mut DbPool<'_>,
) -> LemmyResult<()> {
// Posts
Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), true).await?;
Post::update_removed_for_creator(pool, banned_person_id, Some(community_id), remove).await?;

// Comments
// TODO Diesel doesn't allow updates with joins, so this has to be a loop
Expand All @@ -808,7 +825,7 @@ pub async fn remove_user_data_in_community(
pool,
comment_id,
&CommentUpdateForm {
removed: Some(true),
removed: Some(remove),
..Default::default()
},
)
Expand Down
11 changes: 8 additions & 3 deletions crates/apub/src/activities/block/block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use anyhow::anyhow;
use chrono::{DateTime, Utc};
use lemmy_api_common::{
context::LemmyContext,
utils::{remove_user_data, remove_user_data_in_community},
utils::{remove_or_restore_user_data_in_community, remove_user_data},
};
use lemmy_db_schema::{
source::{
Expand Down Expand Up @@ -205,8 +205,13 @@ impl ActivityHandler for BlockUser {
.ok();

if self.remove_data.unwrap_or(false) {
remove_user_data_in_community(community.id, blocked_person.id, &mut context.pool())
.await?;
remove_or_restore_user_data_in_community(
community.id,
blocked_person.id,
true,
&mut context.pool(),
)
.await?;
}

// write to mod log
Expand Down
8 changes: 5 additions & 3 deletions crates/apub/src/activities/block/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub(crate) async fn send_ban_from_site(
moderator: Person,
banned_user: Person,
reason: Option<String>,
remove_data: Option<bool>,
remove_or_restore_data: Option<bool>,
ban: bool,
expires: Option<i64>,
context: Data<LemmyContext>,
Expand All @@ -151,7 +151,7 @@ pub(crate) async fn send_ban_from_site(
&site,
&banned_user.into(),
&moderator.into(),
remove_data.unwrap_or(false),
remove_or_restore_data.unwrap_or(false),
reason.clone(),
expires,
&context,
Expand All @@ -162,6 +162,7 @@ pub(crate) async fn send_ban_from_site(
&site,
&banned_user.into(),
&moderator.into(),
remove_or_restore_data.unwrap_or(false),
reason.clone(),
&context,
)
Expand Down Expand Up @@ -190,7 +191,7 @@ pub(crate) async fn send_ban_from_community(
&SiteOrCommunity::Community(community),
&banned_person.into(),
&mod_.into(),
data.remove_data.unwrap_or(false),
data.remove_or_restore_data.unwrap_or(false),
data.reason.clone(),
expires,
&context,
Expand All @@ -201,6 +202,7 @@ pub(crate) async fn send_ban_from_community(
&SiteOrCommunity::Community(community),
&banned_person.into(),
&mod_.into(),
data.remove_or_restore_data.unwrap_or(false),
data.reason.clone(),
&context,
)
Expand Down
21 changes: 20 additions & 1 deletion crates/apub/src/activities/block/undo_block_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ use activitypub_federation::{
protocol::verification::verify_domains_match,
traits::{ActivityHandler, Actor},
};
use lemmy_api_common::context::LemmyContext;
use lemmy_api_common::{
context::LemmyContext,
utils::{remove_or_restore_user_data_in_community, restore_user_data},
};
use lemmy_db_schema::{
source::{
activity::ActivitySendTargets,
Expand All @@ -36,6 +39,7 @@ impl UndoBlockUser {
target: &SiteOrCommunity,
user: &ApubPerson,
mod_: &ApubPerson,
restore_data: bool,
reason: Option<String>,
context: &Data<LemmyContext>,
) -> LemmyResult<()> {
Expand All @@ -58,6 +62,7 @@ impl UndoBlockUser {
kind: UndoType::Undo,
id: id.clone(),
audience,
restore_data: Some(restore_data),
};

let mut inboxes = ActivitySendTargets::to_inbox(user.shared_inbox_or_inbox());
Expand Down Expand Up @@ -114,6 +119,10 @@ impl ActivityHandler for UndoBlockUser {
)
.await?;

if self.restore_data.unwrap_or(false) {
restore_user_data(blocked_person.id, context).await?;
}

// write mod log
let form = ModBanForm {
mod_person_id: mod_person.id,
Expand All @@ -132,6 +141,16 @@ impl ActivityHandler for UndoBlockUser {
};
CommunityPersonBan::unban(&mut context.pool(), &community_user_ban_form).await?;

if self.restore_data.unwrap_or(false) {
remove_or_restore_user_data_in_community(
community.id,
blocked_person.id,
false,
&mut context.pool(),
)
.await?;
}

// write to mod log
let form = ModBanFromCommunityForm {
mod_person_id: mod_person.id,
Expand Down
4 changes: 2 additions & 2 deletions crates/apub/src/activities/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,15 @@ pub async fn match_outgoing_activities(
moderator,
banned_user,
reason,
remove_data,
remove_or_restore_data,
ban,
expires,
} => {
send_ban_from_site(
moderator,
banned_user,
reason,
remove_data,
remove_or_restore_data,
ban,
expires,
context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ pub struct UndoBlockUser {
pub(crate) kind: UndoType,
pub(crate) id: Url,
pub(crate) audience: Option<ObjectId<ApubCommunity>>,

/// Quick and dirty solution.
/// TODO: send a separate Delete activity instead
pub(crate) restore_data: Option<bool>,
}

#[async_trait::async_trait]
Expand Down