From 7fd14b3d2ab9572db838932a7adc5ee0d8184afc Mon Sep 17 00:00:00 2001 From: Nutomic Date: Mon, 28 Aug 2023 12:23:45 +0200 Subject: [PATCH] Make remove content optional during account deletion (fixes #1617) (#3817) * Make remove content optional during account deletion (fixes #1617) * simplify purge params by passing context * update js client * add delete content * update woodpecker config --------- Co-authored-by: Dessalines --- .woodpecker.yml | 4 +- api_tests/package.json | 2 +- api_tests/src/shared.ts | 1 + api_tests/yarn.lock | 8 +-- crates/api/src/local_user/ban_person.rs | 8 +-- crates/api/src/site/purge/community.rs | 16 +---- crates/api/src/site/purge/person.rs | 16 +---- crates/api/src/site/purge/post.rs | 8 +-- crates/api_common/src/person.rs | 1 + crates/api_common/src/request.rs | 12 ++-- crates/api_common/src/send_activity.rs | 2 +- crates/api_common/src/utils.rs | 61 +++++++------------ crates/api_crud/src/user/delete.rs | 11 +++- .../apub/src/activities/block/block_user.rs | 8 +-- .../src/activities/deletion/delete_user.rs | 28 ++++----- crates/apub/src/activities/mod.rs | 2 +- .../activities/deletion/delete_user.rs | 2 + 17 files changed, 71 insertions(+), 119 deletions(-) diff --git a/.woodpecker.yml b/.woodpecker.yml index 0c7efbd85..88bf3be0c 100644 --- a/.woodpecker.yml +++ b/.woodpecker.yml @@ -10,9 +10,9 @@ variables: - "**/Cargo.toml" - "Cargo.lock" # database migrations - - "migrations" + - "migrations/**" # typescript tests - - "api_tests" + - "api_tests/**" # config files and scripts used by ci - ".woodpecker.yml" - ".rustfmt.toml" diff --git a/api_tests/package.json b/api_tests/package.json index 1810b8d2f..61c11d00a 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -19,7 +19,7 @@ "eslint": "^8.40.0", "eslint-plugin-prettier": "^4.0.0", "jest": "^29.5.0", - "lemmy-js-client": "0.18.3-rc.3", + "lemmy-js-client": "0.19.0-rc.2", "prettier": "^3.0.0", "ts-jest": "^29.1.0", "typescript": "^5.0.4" diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 6110f1e34..bc760fd33 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -707,6 +707,7 @@ export async function getPersonDetails( export async function deleteUser(api: API): Promise { let form: DeleteAccount = { auth: api.auth, + delete_content: true, password, }; return api.client.deleteAccount(form); diff --git a/api_tests/yarn.lock b/api_tests/yarn.lock index 275dcd067..b243c1b65 100644 --- a/api_tests/yarn.lock +++ b/api_tests/yarn.lock @@ -2174,10 +2174,10 @@ kleur@^3.0.3: resolved "https://registry.yarnpkg.com/kleur/-/kleur-3.0.3.tgz#a79c9ecc86ee1ce3fa6206d1216c501f147fc07e" integrity sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w== -lemmy-js-client@0.18.3-rc.3: - version "0.18.3-rc.3" - resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.18.3-rc.3.tgz#fc6489eb141bd09558bca38d9e46b40771a29f37" - integrity sha512-njixgXk4uMU4gGifnljwhSe9Kf445C4wAXcXhtpTtwPPLXpHQgxA1RASMb9Uq4zblfE6nC2JbrAka8y8N2N/Bw== +lemmy-js-client@0.19.0-rc.2: + version "0.19.0-rc.2" + resolved "https://registry.yarnpkg.com/lemmy-js-client/-/lemmy-js-client-0.19.0-rc.2.tgz#c3cb511b27f92538909a2b91a0f8527b1abad958" + integrity sha512-FXuf8s7bpBVkHL/OGWDb/0aGIrJ7uv3d4Xt1h6zmNDhw6MmmuD8RXgCHiS2jqhxjAEp96Dpl1NFXbpmKpix7tQ== dependencies: cross-fetch "^3.1.5" form-data "^4.0.0" diff --git a/crates/api/src/local_user/ban_person.rs b/crates/api/src/local_user/ban_person.rs index 490a51448..3a3a240da 100644 --- a/crates/api/src/local_user/ban_person.rs +++ b/crates/api/src/local_user/ban_person.rs @@ -47,13 +47,7 @@ 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, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; + remove_user_data(person.id, &context).await?; } // Mod tables diff --git a/crates/api/src/site/purge/community.rs b/crates/api/src/site/purge/community.rs index bd8d9d386..7c59fd76b 100644 --- a/crates/api/src/site/purge/community.rs +++ b/crates/api/src/site/purge/community.rs @@ -33,24 +33,14 @@ impl Perform for PurgeCommunity { let community = Community::read(&mut context.pool(), community_id).await?; if let Some(banner) = community.banner { - purge_image_from_pictrs(context.client(), context.settings(), &banner) - .await - .ok(); + purge_image_from_pictrs(&banner, context).await.ok(); } if let Some(icon) = community.icon { - purge_image_from_pictrs(context.client(), context.settings(), &icon) - .await - .ok(); + purge_image_from_pictrs(&icon, context).await.ok(); } - purge_image_posts_for_community( - community_id, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; + purge_image_posts_for_community(community_id, context).await?; Community::delete(&mut context.pool(), community_id).await?; diff --git a/crates/api/src/site/purge/person.rs b/crates/api/src/site/purge/person.rs index 838b36070..1100a0db9 100644 --- a/crates/api/src/site/purge/person.rs +++ b/crates/api/src/site/purge/person.rs @@ -32,24 +32,14 @@ impl Perform for PurgePerson { let person = Person::read(&mut context.pool(), person_id).await?; if let Some(banner) = person.banner { - purge_image_from_pictrs(context.client(), context.settings(), &banner) - .await - .ok(); + purge_image_from_pictrs(&banner, context).await.ok(); } if let Some(avatar) = person.avatar { - purge_image_from_pictrs(context.client(), context.settings(), &avatar) - .await - .ok(); + purge_image_from_pictrs(&avatar, context).await.ok(); } - purge_image_posts_for_person( - person_id, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; + purge_image_posts_for_person(person_id, context).await?; Person::delete(&mut context.pool(), person_id).await?; diff --git a/crates/api/src/site/purge/post.rs b/crates/api/src/site/purge/post.rs index ee0a3af09..b267f3518 100644 --- a/crates/api/src/site/purge/post.rs +++ b/crates/api/src/site/purge/post.rs @@ -34,15 +34,11 @@ impl Perform for PurgePost { // Purge image if let Some(url) = post.url { - purge_image_from_pictrs(context.client(), context.settings(), &url) - .await - .ok(); + purge_image_from_pictrs(&url, context).await.ok(); } // Purge thumbnail if let Some(thumbnail_url) = post.thumbnail_url { - purge_image_from_pictrs(context.client(), context.settings(), &thumbnail_url) - .await - .ok(); + purge_image_from_pictrs(&thumbnail_url, context).await.ok(); } let community_id = post.community_id; diff --git a/crates/api_common/src/person.rs b/crates/api_common/src/person.rs index ef970f1bc..695486a5a 100644 --- a/crates/api_common/src/person.rs +++ b/crates/api_common/src/person.rs @@ -365,6 +365,7 @@ pub struct CommentReplyResponse { /// Delete your account. pub struct DeleteAccount { pub password: Sensitive, + pub delete_content: bool, pub auth: Sensitive, } diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index b62514c02..8fe302741 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -1,4 +1,4 @@ -use crate::post::SiteMetadata; +use crate::{context::LemmyContext, post::SiteMetadata}; use encoding::{all::encodings, DecoderTrap}; use lemmy_db_schema::newtypes::DbUrl; use lemmy_utils::{ @@ -150,12 +150,11 @@ pub(crate) async fn fetch_pictrs( /// - It might not be an image /// - Pictrs might not be set up pub async fn purge_image_from_pictrs( - client: &ClientWithMiddleware, - settings: &Settings, image_url: &Url, + context: &LemmyContext, ) -> Result<(), LemmyError> { - let pictrs_config = settings.pictrs_config()?; - is_image_content_type(client, image_url).await?; + let pictrs_config = context.settings().pictrs_config()?; + is_image_content_type(context.client(), image_url).await?; let alias = image_url .path_segments() @@ -168,7 +167,8 @@ pub async fn purge_image_from_pictrs( let pictrs_api_key = pictrs_config .api_key .ok_or(LemmyErrorType::PictrsApiKeyNotProvided)?; - let response = client + let response = context + .client() .post(&purge_url) .timeout(REQWEST_TIMEOUT) .header("x-api-token", pictrs_api_key) diff --git a/crates/api_common/src/send_activity.rs b/crates/api_common/src/send_activity.rs index 1ba9aa646..45109d5c5 100644 --- a/crates/api_common/src/send_activity.rs +++ b/crates/api_common/src/send_activity.rs @@ -58,7 +58,7 @@ pub enum SendActivityData { CreatePrivateMessage(PrivateMessageView), UpdatePrivateMessage(PrivateMessageView), DeletePrivateMessage(Person, PrivateMessage, bool), - DeleteUser(Person), + DeleteUser(Person, bool), CreateReport(Url, Person, Community, String), } diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index a79e3bd52..63095f41a 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -42,7 +42,6 @@ use lemmy_utils::{ utils::slurs::build_slur_regex, }; use regex::Regex; -use reqwest_middleware::ClientWithMiddleware; use rosetta_i18n::{Language, LanguageId}; use tracing::warn; use url::{ParseError, Url}; @@ -529,19 +528,16 @@ pub fn check_private_instance_and_federation_enabled( pub async fn purge_image_posts_for_person( banned_person_id: PersonId, - pool: &mut DbPool<'_>, - settings: &Settings, - client: &ClientWithMiddleware, + context: &LemmyContext, ) -> Result<(), LemmyError> { + let pool = &mut context.pool(); let posts = Post::fetch_pictrs_posts_for_creator(pool, banned_person_id).await?; for post in posts { if let Some(url) = post.url { - purge_image_from_pictrs(client, settings, &url).await.ok(); + purge_image_from_pictrs(&url, context).await.ok(); } if let Some(thumbnail_url) = post.thumbnail_url { - purge_image_from_pictrs(client, settings, &thumbnail_url) - .await - .ok(); + purge_image_from_pictrs(&thumbnail_url, context).await.ok(); } } @@ -552,19 +548,16 @@ pub async fn purge_image_posts_for_person( pub async fn purge_image_posts_for_community( banned_community_id: CommunityId, - pool: &mut DbPool<'_>, - settings: &Settings, - client: &ClientWithMiddleware, + context: &LemmyContext, ) -> Result<(), LemmyError> { + let pool = &mut context.pool(); let posts = Post::fetch_pictrs_posts_for_community(pool, banned_community_id).await?; for post in posts { if let Some(url) = post.url { - purge_image_from_pictrs(client, settings, &url).await.ok(); + purge_image_from_pictrs(&url, context).await.ok(); } if let Some(thumbnail_url) = post.thumbnail_url { - purge_image_from_pictrs(client, settings, &thumbnail_url) - .await - .ok(); + purge_image_from_pictrs(&thumbnail_url, context).await.ok(); } } @@ -575,21 +568,16 @@ pub async fn purge_image_posts_for_community( pub async fn remove_user_data( banned_person_id: PersonId, - pool: &mut DbPool<'_>, - settings: &Settings, - client: &ClientWithMiddleware, + context: &LemmyContext, ) -> Result<(), LemmyError> { + let pool = &mut context.pool(); // Purge user images let person = Person::read(pool, banned_person_id).await?; if let Some(avatar) = person.avatar { - purge_image_from_pictrs(client, settings, &avatar) - .await - .ok(); + purge_image_from_pictrs(&avatar, context).await.ok(); } if let Some(banner) = person.banner { - purge_image_from_pictrs(client, settings, &banner) - .await - .ok(); + purge_image_from_pictrs(&banner, context).await.ok(); } // Update the fields to None @@ -608,7 +596,7 @@ pub async fn remove_user_data( Post::update_removed_for_creator(pool, banned_person_id, None, true).await?; // Purge image posts - purge_image_posts_for_person(banned_person_id, pool, settings, client).await?; + purge_image_posts_for_person(banned_person_id, context).await?; // Communities // Remove all communities where they're the top mod @@ -635,12 +623,10 @@ pub async fn remove_user_data( // Delete the community images if let Some(icon) = first_mod_community.community.icon { - purge_image_from_pictrs(client, settings, &icon).await.ok(); + purge_image_from_pictrs(&icon, context).await.ok(); } if let Some(banner) = first_mod_community.community.banner { - purge_image_from_pictrs(client, settings, &banner) - .await - .ok(); + purge_image_from_pictrs(&banner, context).await.ok(); } // Update the fields to None Community::update( @@ -695,23 +681,18 @@ pub async fn remove_user_data_in_community( Ok(()) } -pub async fn delete_user_account( +pub async fn purge_user_account( person_id: PersonId, - pool: &mut DbPool<'_>, - settings: &Settings, - client: &ClientWithMiddleware, + context: &LemmyContext, ) -> Result<(), LemmyError> { + let pool = &mut context.pool(); // Delete their images let person = Person::read(pool, person_id).await?; if let Some(avatar) = person.avatar { - purge_image_from_pictrs(client, settings, &avatar) - .await - .ok(); + purge_image_from_pictrs(&avatar, context).await.ok(); } if let Some(banner) = person.banner { - purge_image_from_pictrs(client, settings, &banner) - .await - .ok(); + purge_image_from_pictrs(&banner, context).await.ok(); } // No need to update avatar and banner, those are handled in Person::delete_account @@ -726,7 +707,7 @@ pub async fn delete_user_account( .with_lemmy_type(LemmyErrorType::CouldntUpdatePost)?; // Purge image posts - purge_image_posts_for_person(person_id, pool, settings, client).await?; + purge_image_posts_for_person(person_id, context).await?; // Leave communities they mod CommunityModerator::leave_all_communities(pool, person_id).await?; diff --git a/crates/api_crud/src/user/delete.rs b/crates/api_crud/src/user/delete.rs index 94c547b14..20796c34b 100644 --- a/crates/api_crud/src/user/delete.rs +++ b/crates/api_crud/src/user/delete.rs @@ -5,8 +5,9 @@ use lemmy_api_common::{ context::LemmyContext, person::{DeleteAccount, DeleteAccountResponse}, send_activity::{ActivityChannel, SendActivityData}, - utils::local_user_view_from_jwt, + utils::{local_user_view_from_jwt, purge_user_account}, }; +use lemmy_db_schema::source::person::Person; use lemmy_utils::error::{LemmyError, LemmyErrorType}; #[tracing::instrument(skip(context))] @@ -26,8 +27,14 @@ pub async fn delete_account( return Err(LemmyErrorType::IncorrectLogin)?; } + if data.delete_content { + purge_user_account(local_user_view.person.id, &context).await?; + } else { + Person::delete_account(&mut context.pool(), local_user_view.person.id).await?; + } + ActivityChannel::submit_activity( - SendActivityData::DeleteUser(local_user_view.person), + SendActivityData::DeleteUser(local_user_view.person, data.delete_content), &context, ) .await?; diff --git a/crates/apub/src/activities/block/block_user.rs b/crates/apub/src/activities/block/block_user.rs index 163aca038..7b14213ca 100644 --- a/crates/apub/src/activities/block/block_user.rs +++ b/crates/apub/src/activities/block/block_user.rs @@ -165,13 +165,7 @@ impl ActivityHandler for BlockUser { ) .await?; if self.remove_data.unwrap_or(false) { - remove_user_data( - blocked_person.id, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; + remove_user_data(blocked_person.id, context).await?; } // write mod log diff --git a/crates/apub/src/activities/deletion/delete_user.rs b/crates/apub/src/activities/deletion/delete_user.rs index cf37dc5ab..66a402161 100644 --- a/crates/apub/src/activities/deletion/delete_user.rs +++ b/crates/apub/src/activities/deletion/delete_user.rs @@ -10,20 +10,17 @@ use activitypub_federation::{ protocol::verification::verify_urls_match, traits::{ActivityHandler, Actor}, }; -use lemmy_api_common::{context::LemmyContext, utils::delete_user_account}; +use lemmy_api_common::{context::LemmyContext, utils::purge_user_account}; use lemmy_db_schema::source::person::Person; use lemmy_utils::error::LemmyError; use url::Url; -pub async fn delete_user(person: Person, context: Data) -> Result<(), LemmyError> { +pub async fn delete_user( + person: Person, + delete_content: bool, + context: Data, +) -> Result<(), LemmyError> { let actor: ApubPerson = person.into(); - delete_user_account( - actor.id, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; let id = generate_activity_id( DeleteType::Delete, @@ -36,6 +33,7 @@ pub async fn delete_user(person: Person, context: Data) -> Result< kind: DeleteType::Delete, id: id.clone(), cc: vec![], + remove_data: Some(delete_content), }; let inboxes = remote_instance_inboxes(&mut context.pool()).await?; @@ -68,13 +66,11 @@ impl ActivityHandler for DeleteUser { async fn receive(self, context: &Data) -> Result<(), LemmyError> { let actor = self.actor.dereference(context).await?; - delete_user_account( - actor.id, - &mut context.pool(), - context.settings(), - context.client(), - ) - .await?; + if self.remove_data.unwrap_or(false) { + purge_user_account(actor.id, context).await?; + } else { + Person::delete_account(&mut context.pool(), actor.id).await?; + } Ok(()) } } diff --git a/crates/apub/src/activities/mod.rs b/crates/apub/src/activities/mod.rs index ad04e861f..2fe0a82d1 100644 --- a/crates/apub/src/activities/mod.rs +++ b/crates/apub/src/activities/mod.rs @@ -335,7 +335,7 @@ pub async fn match_outgoing_activities( DeletePrivateMessage(person, pm, deleted) => { send_apub_delete_private_message(&person.into(), pm, deleted, context).await } - DeleteUser(person) => delete_user(person, context).await, + DeleteUser(person, delete_content) => delete_user(person, delete_content, context).await, CreateReport(url, actor, community, reason) => { Report::send(ObjectId::from(url), actor, community, reason, context).await } diff --git a/crates/apub/src/protocol/activities/deletion/delete_user.rs b/crates/apub/src/protocol/activities/deletion/delete_user.rs index f5f9908d5..46b070fab 100644 --- a/crates/apub/src/protocol/activities/deletion/delete_user.rs +++ b/crates/apub/src/protocol/activities/deletion/delete_user.rs @@ -23,4 +23,6 @@ pub struct DeleteUser { #[serde(deserialize_with = "deserialize_one_or_many", default)] #[serde(skip_serializing_if = "Vec::is_empty")] pub(crate) cc: Vec, + /// Nonstandard field. If present, all content from the user should be deleted along with the account + pub(crate) remove_data: Option, }