From fe3ebea95a3439f30a25882607888f8bd2d8b819 Mon Sep 17 00:00:00 2001 From: Anon Date: Wed, 6 Sep 2023 08:13:30 -0500 Subject: [PATCH] Add logging for pictrs uploads (#3927) * Add logging for pictrs uploads * cleanup --- crates/api/src/site/purge/person.rs | 23 ++++----- crates/api_common/src/request.rs | 31 +++++++++++- crates/db_schema/src/impls/image_upload.rs | 47 +++++++++++++++++++ crates/db_schema/src/impls/mod.rs | 1 + crates/db_schema/src/newtypes.rs | 6 +++ crates/db_schema/src/schema.rs | 14 +++++- crates/db_schema/src/source/image_upload.rs | 36 ++++++++++++++ crates/db_schema/src/source/mod.rs | 1 + crates/routes/src/images.rs | 30 ++++++++++-- .../down.sql | 2 + .../2023-08-31-205559_add_image_upload/up.sql | 10 ++++ scripts/sql_format_check.sh | 2 +- 12 files changed, 184 insertions(+), 19 deletions(-) create mode 100644 crates/db_schema/src/impls/image_upload.rs create mode 100644 crates/db_schema/src/source/image_upload.rs create mode 100644 migrations/2023-08-31-205559_add_image_upload/down.sql create mode 100644 migrations/2023-08-31-205559_add_image_upload/up.sql diff --git a/crates/api/src/site/purge/person.rs b/crates/api/src/site/purge/person.rs index 3c6797118..ed38e2354 100644 --- a/crates/api/src/site/purge/person.rs +++ b/crates/api/src/site/purge/person.rs @@ -1,17 +1,19 @@ use actix_web::web::{Data, Json}; use lemmy_api_common::{ context::LemmyContext, - request::purge_image_from_pictrs, + request::delete_image_from_pictrs, site::{PurgeItemResponse, PurgePerson}, - utils::{is_admin, local_user_view_from_jwt, purge_image_posts_for_person, sanitize_html_opt}, + utils::{is_admin, local_user_view_from_jwt, sanitize_html_opt}, }; use lemmy_db_schema::{ source::{ + image_upload::ImageUpload, moderator::{AdminPurgePerson, AdminPurgePersonForm}, person::Person, }, traits::Crud, }; +use lemmy_db_views::structs::LocalUserView; use lemmy_utils::error::LemmyError; #[tracing::instrument(skip(context))] @@ -26,18 +28,17 @@ pub async fn purge_person( // Read the person to get their images let person_id = data.person_id; - let person = Person::read(&mut context.pool(), person_id).await?; - if let Some(banner) = person.banner { - purge_image_from_pictrs(&banner, &context).await.ok(); + let local_user = LocalUserView::read_person(&mut context.pool(), person_id).await?; + let pictrs_uploads = + ImageUpload::get_all_by_local_user_id(&mut context.pool(), &local_user.local_user.id).await?; + + for upload in pictrs_uploads { + delete_image_from_pictrs(&upload.pictrs_alias, &upload.pictrs_delete_token, &context) + .await + .ok(); } - if let Some(avatar) = person.avatar { - purge_image_from_pictrs(&avatar, &context).await.ok(); - } - - purge_image_posts_for_person(person_id, &context).await?; - Person::delete(&mut context.pool(), person_id).await?; // Mod tables diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index 55a9be1ff..b27bf2e92 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -158,7 +158,6 @@ pub async fn purge_image_from_pictrs( image_url: &Url, context: &LemmyContext, ) -> Result<(), LemmyError> { - let pictrs_config = context.settings().pictrs_config()?; is_image_content_type(context.client(), image_url).await?; let alias = image_url @@ -167,7 +166,15 @@ pub async fn purge_image_from_pictrs( .next_back() .ok_or(LemmyErrorType::ImageUrlMissingLastPathSegment)?; - let purge_url = format!("{}/internal/purge?alias={}", pictrs_config.url, alias); + purge_image_from_pictrs_by_alias(alias, context).await +} + +pub async fn purge_image_from_pictrs_by_alias( + alias: &str, + context: &LemmyContext, +) -> Result<(), LemmyError> { + let pictrs_config = context.settings().pictrs_config()?; + let purge_url = format!("{}internal/purge?alias={}", pictrs_config.url, alias); let pictrs_api_key = pictrs_config .api_key @@ -189,6 +196,26 @@ pub async fn purge_image_from_pictrs( } } +pub async fn delete_image_from_pictrs( + alias: &str, + delete_token: &str, + context: &LemmyContext, +) -> Result<(), LemmyError> { + let pictrs_config = context.settings().pictrs_config()?; + let url = format!( + "{}image/delete/{}/{}", + pictrs_config.url, &delete_token, &alias + ); + context + .client() + .delete(&url) + .timeout(REQWEST_TIMEOUT) + .send() + .await + .map_err(LemmyError::from)?; + Ok(()) +} + /// Both are options, since the URL might be either an html page, or an image /// Returns the SiteMetadata, and an image URL, if there is a picture associated #[tracing::instrument(skip_all)] diff --git a/crates/db_schema/src/impls/image_upload.rs b/crates/db_schema/src/impls/image_upload.rs new file mode 100644 index 000000000..58edbf2e3 --- /dev/null +++ b/crates/db_schema/src/impls/image_upload.rs @@ -0,0 +1,47 @@ +use crate::{ + newtypes::{ImageUploadId, LocalUserId}, + schema::image_upload::dsl::{image_upload, local_user_id, pictrs_alias}, + source::image_upload::{ImageUpload, ImageUploadForm}, + utils::{get_conn, DbPool}, +}; +use diesel::{insert_into, result::Error, ExpressionMethods, QueryDsl, Table}; +use diesel_async::RunQueryDsl; + +impl ImageUpload { + pub async fn create(pool: &mut DbPool<'_>, form: &ImageUploadForm) -> Result { + let conn = &mut get_conn(pool).await?; + insert_into(image_upload) + .values(form) + .get_result::(conn) + .await + } + + pub async fn get_all_by_local_user_id( + pool: &mut DbPool<'_>, + user_id: &LocalUserId, + ) -> Result, Error> { + let conn = &mut get_conn(pool).await?; + image_upload + .filter(local_user_id.eq(user_id)) + .select(image_upload::all_columns()) + .load::(conn) + .await + } + + pub async fn delete( + pool: &mut DbPool<'_>, + image_upload_id: ImageUploadId, + ) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::delete(image_upload.find(image_upload_id)) + .execute(conn) + .await + } + + pub async fn delete_by_alias(pool: &mut DbPool<'_>, alias: &str) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::delete(image_upload.filter(pictrs_alias.eq(alias))) + .execute(conn) + .await + } +} diff --git a/crates/db_schema/src/impls/mod.rs b/crates/db_schema/src/impls/mod.rs index f13004d01..a8aabf18d 100644 --- a/crates/db_schema/src/impls/mod.rs +++ b/crates/db_schema/src/impls/mod.rs @@ -10,6 +10,7 @@ pub mod custom_emoji; pub mod email_verification; pub mod federation_allowlist; pub mod federation_blocklist; +pub mod image_upload; pub mod instance; pub mod language; pub mod local_site; diff --git a/crates/db_schema/src/newtypes.rs b/crates/db_schema/src/newtypes.rs index f5958105c..af652d219 100644 --- a/crates/db_schema/src/newtypes.rs +++ b/crates/db_schema/src/newtypes.rs @@ -137,6 +137,12 @@ pub struct CommunityLanguageId(pub i32); /// The comment reply id. pub struct CommentReplyId(i32); +#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize, Default)] +#[cfg_attr(feature = "full", derive(DieselNewType, TS))] +#[cfg_attr(feature = "full", ts(export))] +/// The Image Upload id. +pub struct ImageUploadId(i32); + #[derive(Debug, Copy, Clone, Hash, Eq, PartialEq, Serialize, Deserialize, Default)] #[cfg_attr(feature = "full", derive(DieselNewType, TS))] #[cfg_attr(feature = "full", ts(export))] diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index ffcdbebe8..e5901200f 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -299,6 +299,16 @@ diesel::table! { } } +diesel::table! { + image_upload (id) { + id -> Int4, + local_user_id -> Int4, + pictrs_alias -> Text, + pictrs_delete_token -> Text, + published -> Timestamptz, + } +} + diesel::table! { instance (id) { id -> Int4, @@ -405,9 +415,9 @@ diesel::table! { totp_2fa_secret -> Nullable, totp_2fa_url -> Nullable, open_links_in_new_tab -> Bool, + infinite_scroll_enabled -> Bool, blur_nsfw -> Bool, auto_expand -> Bool, - infinite_scroll_enabled -> Bool, admin -> Bool, post_listing_mode -> PostListingModeEnum, } @@ -893,6 +903,7 @@ diesel::joinable!(custom_emoji_keyword -> custom_emoji (custom_emoji_id)); diesel::joinable!(email_verification -> local_user (local_user_id)); diesel::joinable!(federation_allowlist -> instance (instance_id)); diesel::joinable!(federation_blocklist -> instance (instance_id)); +diesel::joinable!(image_upload -> local_user (local_user_id)); diesel::joinable!(local_site -> site (site_id)); diesel::joinable!(local_site_rate_limit -> local_site (local_site_id)); diesel::joinable!(local_user -> person (person_id)); @@ -967,6 +978,7 @@ diesel::allow_tables_to_appear_in_same_query!( email_verification, federation_allowlist, federation_blocklist, + image_upload, instance, language, local_site, diff --git a/crates/db_schema/src/source/image_upload.rs b/crates/db_schema/src/source/image_upload.rs new file mode 100644 index 000000000..0a3c4d6c4 --- /dev/null +++ b/crates/db_schema/src/source/image_upload.rs @@ -0,0 +1,36 @@ +use crate::newtypes::{ImageUploadId, LocalUserId}; +#[cfg(feature = "full")] +use crate::schema::image_upload; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use serde_with::skip_serializing_none; +use std::fmt::Debug; +#[cfg(feature = "full")] +use ts_rs::TS; +use typed_builder::TypedBuilder; + +#[skip_serializing_none] +#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] +#[cfg_attr(feature = "full", derive(Queryable, Associations, Identifiable, TS))] +#[cfg_attr(feature = "full", diesel(table_name = image_upload))] +#[cfg_attr(feature = "full", ts(export))] +#[cfg_attr( + feature = "full", + diesel(belongs_to(crate::source::local_user::LocalUser)) +)] +pub struct ImageUpload { + pub id: ImageUploadId, + pub local_user_id: LocalUserId, + pub pictrs_alias: String, + pub pictrs_delete_token: String, + pub published: DateTime, +} + +#[derive(Debug, Clone, TypedBuilder)] +#[cfg_attr(feature = "full", derive(Insertable, AsChangeset))] +#[cfg_attr(feature = "full", diesel(table_name = image_upload))] +pub struct ImageUploadForm { + pub local_user_id: LocalUserId, + pub pictrs_alias: String, + pub pictrs_delete_token: String, +} diff --git a/crates/db_schema/src/source/mod.rs b/crates/db_schema/src/source/mod.rs index a46f4fb40..a200806cd 100644 --- a/crates/db_schema/src/source/mod.rs +++ b/crates/db_schema/src/source/mod.rs @@ -15,6 +15,7 @@ pub mod custom_emoji_keyword; pub mod email_verification; pub mod federation_allowlist; pub mod federation_blocklist; +pub mod image_upload; pub mod instance; pub mod language; pub mod local_site; diff --git a/crates/routes/src/images.rs b/crates/routes/src/images.rs index c81a56602..5133cfd82 100644 --- a/crates/routes/src/images.rs +++ b/crates/routes/src/images.rs @@ -12,7 +12,13 @@ use actix_web::{ }; use futures::stream::{Stream, StreamExt}; use lemmy_api_common::{context::LemmyContext, utils::local_user_view_from_jwt}; -use lemmy_db_schema::source::local_site::LocalSite; +use lemmy_db_schema::{ + newtypes::LocalUserId, + source::{ + image_upload::{ImageUpload, ImageUploadForm}, + local_site::LocalSite, + }, +}; use lemmy_utils::{claims::Claims, rate_limit::RateLimitCell, REQWEST_TIMEOUT}; use reqwest::Body; use reqwest_middleware::{ClientWithMiddleware, RequestBuilder}; @@ -95,8 +101,8 @@ async fn upload( let jwt = req.cookie("jwt").ok_or(error::ErrorUnauthorized( "No auth header for picture upload", ))?; - - if Claims::decode(jwt.value(), &context.secret().jwt_secret).is_err() { + let claims = Claims::decode(jwt.value(), &context.secret().jwt_secret); + if claims.is_err() { return Ok(HttpResponse::Unauthorized().finish()); }; @@ -108,7 +114,6 @@ async fn upload( if let Some(addr) = req.head().peer_addr { client_req = client_req.header("X-Forwarded-For", addr.to_string()) }; - let res = client_req .body(Body::wrap_stream(make_send(body))) .send() @@ -117,6 +122,19 @@ async fn upload( let status = res.status(); let images = res.json::().await.map_err(error::ErrorBadRequest)?; + if let Some(images) = &images.files { + let local_user_id = LocalUserId(claims?.claims.sub); + for uploaded_image in images { + let form = ImageUploadForm { + local_user_id, + pictrs_alias: uploaded_image.file.to_string(), + pictrs_delete_token: uploaded_image.delete_token.to_string(), + }; + ImageUpload::create(&mut context.pool(), &form) + .await + .map_err(error::ErrorBadRequest)?; + } + } Ok(HttpResponse::build(status).json(images)) } @@ -215,6 +233,10 @@ async fn delete( let res = client_req.send().await.map_err(error::ErrorBadRequest)?; + ImageUpload::delete_by_alias(&mut context.pool(), &file) + .await + .map_err(error::ErrorBadRequest)?; + Ok(HttpResponse::build(res.status()).body(BodyStream::new(res.bytes_stream()))) } diff --git a/migrations/2023-08-31-205559_add_image_upload/down.sql b/migrations/2023-08-31-205559_add_image_upload/down.sql new file mode 100644 index 000000000..e042894fc --- /dev/null +++ b/migrations/2023-08-31-205559_add_image_upload/down.sql @@ -0,0 +1,2 @@ +DROP TABLE image_upload; + diff --git a/migrations/2023-08-31-205559_add_image_upload/up.sql b/migrations/2023-08-31-205559_add_image_upload/up.sql new file mode 100644 index 000000000..eabd14d2b --- /dev/null +++ b/migrations/2023-08-31-205559_add_image_upload/up.sql @@ -0,0 +1,10 @@ +CREATE TABLE image_upload ( + id serial PRIMARY KEY, + local_user_id int REFERENCES local_user ON UPDATE CASCADE ON DELETE CASCADE NOT NULL, + pictrs_alias text NOT NULL UNIQUE, + pictrs_delete_token text NOT NULL, + published timestamptz DEFAULT now() NOT NULL +); + +CREATE INDEX idx_image_upload_local_user_id ON image_upload (local_user_id); + diff --git a/scripts/sql_format_check.sh b/scripts/sql_format_check.sh index 8a92eefd0..0178204de 100755 --- a/scripts/sql_format_check.sh +++ b/scripts/sql_format_check.sh @@ -11,5 +11,5 @@ find migrations -type f -name "*.sql" -print0 | while read -d $'\0' FILE do TMP_FILE="/tmp/tmp_pg_format.sql" pg_format $FILE > $TMP_FILE - diff $FILE $TMP_FILE + diff -u $FILE $TMP_FILE done