From 5bc3f0c4d9855c1d6dc42f72ed6b11749289ddef Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 15 Jan 2025 16:28:43 +0000 Subject: [PATCH] Pictrs delete token (#5317) * Split image endpoints into API v3 and v4 * Move into subfolders * Upload avatar endpoint and other changes * Various other changes fixes #1772 fixes #4001 * clippy * config options * fix ts bindings * fix api tests * Add option to disable image upload (fixes #1118) * split files into upload, download * move sitemap to top level, not in api * simplify code * add upload user banner * community icon/banner * site icon/banner * update js client * wip * add delete endpoints * change comment * optimization Co-authored-by: dullbananas * move fn * 1024px banner * dont use static client * fix api tests * shear * proxy pictrs in request.rs (fixes #5270) * clippy * Get rid of pictrs delete token * remove delete token params * try to fix api tests * fmt * skip api tests * clippy * create user * debug * dbg * ignore test * test * image * run another * fixed? * clippy * fix * migration with column order * drop default * fix health check * update client * remove unused * fix * reuse delete_image_from_pictrs * update lib --------- Co-authored-by: dullbananas --- api_tests/package.json | 2 +- api_tests/pnpm-lock.yaml | 10 ++--- api_tests/src/image.spec.ts | 4 -- api_tests/src/shared.ts | 1 - crates/api_common/src/image.rs | 2 - crates/api_common/src/request.rs | 16 ++------ crates/api_common/src/utils.rs | 10 ++--- crates/db_schema/src/impls/images.rs | 20 +++++++++- crates/db_schema/src/schema.rs | 1 - crates/db_schema/src/source/image_upload.rs | 39 ------------------- crates/db_schema/src/source/images.rs | 2 - crates/routes/src/images/delete.rs | 25 +++++------- crates/routes/src/images/upload.rs | 2 - crates/routes/src/images/utils.rs | 2 +- .../2025-01-09-144233_no-image-token/down.sql | 19 +++++++++ .../2025-01-09-144233_no-image-token/up.sql | 3 ++ 16 files changed, 64 insertions(+), 94 deletions(-) delete mode 100644 crates/db_schema/src/source/image_upload.rs create mode 100644 migrations/2025-01-09-144233_no-image-token/down.sql create mode 100644 migrations/2025-01-09-144233_no-image-token/up.sql diff --git a/api_tests/package.json b/api_tests/package.json index 8bed8c8c2..7e1fd8127 100644 --- a/api_tests/package.json +++ b/api_tests/package.json @@ -28,7 +28,7 @@ "eslint": "^9.18.0", "eslint-plugin-prettier": "^5.1.3", "jest": "^29.5.0", - "lemmy-js-client": "0.20.0-modlog-combined.0", + "lemmy-js-client": "0.20.0-no-delete-token.2", "prettier": "^3.4.2", "ts-jest": "^29.1.0", "typescript": "^5.7.3", diff --git a/api_tests/pnpm-lock.yaml b/api_tests/pnpm-lock.yaml index 71c88734f..1a68b2777 100644 --- a/api_tests/pnpm-lock.yaml +++ b/api_tests/pnpm-lock.yaml @@ -30,8 +30,8 @@ importers: specifier: ^29.5.0 version: 29.7.0(@types/node@22.10.6) lemmy-js-client: - specifier: 0.20.0-modlog-combined.0 - version: 0.20.0-modlog-combined.0 + specifier: 0.20.0-no-delete-token.2 + version: 0.20.0-no-delete-token.2 prettier: specifier: ^3.4.2 version: 3.4.2 @@ -1157,8 +1157,8 @@ packages: resolution: {integrity: sha512-eTIzlVOSUR+JxdDFepEYcBMtZ9Qqdef+rnzWdRZuMbOywu5tO2w2N7rqjoANZ5k9vywhL6Br1VRjUIgTQx4E8w==} engines: {node: '>=6'} - lemmy-js-client@0.20.0-modlog-combined.0: - resolution: {integrity: sha512-lb3na39klOSE184hJJObMufKjHtm3Mrk42RHqyVNCYZQ+FAAbQzBFTuYyqv8QJV5TJlMmyFO2v1v/9cH72nLRg==} + lemmy-js-client@0.20.0-no-delete-token.2: + resolution: {integrity: sha512-3ra3DpD8XR6RRwCeUDLI/ztFgVuF1IoUoft+xKVDALyupwRWUsA3JcHXRIcFd1a2Qt+pHJtWbc5Iwvybakxwdg==} leven@3.1.0: resolution: {integrity: sha512-qsda+H8jTaUaN/x5vzW2rzc+8Rw4TAQ/4KjB46IwK5VH+IlVeeeje/EoZRpiXvIqjFgK84QffqPztGI3VBLG1A==} @@ -3060,7 +3060,7 @@ snapshots: kleur@3.0.3: {} - lemmy-js-client@0.20.0-modlog-combined.0: {} + lemmy-js-client@0.20.0-no-delete-token.2: {} leven@3.1.0: {} diff --git a/api_tests/src/image.spec.ts b/api_tests/src/image.spec.ts index 1a686879b..b5cc72fe0 100644 --- a/api_tests/src/image.spec.ts +++ b/api_tests/src/image.spec.ts @@ -55,7 +55,6 @@ test("Upload image and delete it", async () => { const upload = await alphaImage.uploadImage(upload_form); expect(upload.image_url).toBeDefined(); expect(upload.filename).toBeDefined(); - expect(upload.delete_token).toBeDefined(); // ensure that image download is working. theres probably a better way to do this const response = await fetch(upload.image_url ?? ""); @@ -82,7 +81,6 @@ test("Upload image and delete it", async () => { // delete image const delete_form: DeleteImageParams = { - token: upload.delete_token, filename: upload.filename, }; const delete_ = await alphaImage.deleteImage(delete_form); @@ -113,7 +111,6 @@ test("Purge user, uploaded image removed", async () => { }; const upload = await user.uploadImage(upload_form); expect(upload.filename).toBeDefined(); - expect(upload.delete_token).toBeDefined(); expect(upload.image_url).toBeDefined(); // ensure that image download is working. theres probably a better way to do this @@ -144,7 +141,6 @@ test("Purge post, linked image removed", async () => { }; const upload = await user.uploadImage(upload_form); expect(upload.filename).toBeDefined(); - expect(upload.delete_token).toBeDefined(); expect(upload.image_url).toBeDefined(); // ensure that image download is working. theres probably a better way to do this diff --git a/api_tests/src/shared.ts b/api_tests/src/shared.ts index 771b1dc57..6c4b4eba0 100644 --- a/api_tests/src/shared.ts +++ b/api_tests/src/shared.ts @@ -952,7 +952,6 @@ export async function deleteAllImages(api: LemmyHttp) { imagesRes.images .map(image => { const form: DeleteImageParams = { - token: image.local_image.pictrs_delete_token, filename: image.local_image.pictrs_alias, }; return form; diff --git a/crates/api_common/src/image.rs b/crates/api_common/src/image.rs index e93b367f6..9b3e4b00c 100644 --- a/crates/api_common/src/image.rs +++ b/crates/api_common/src/image.rs @@ -21,7 +21,6 @@ pub struct ImageGetParams { #[cfg_attr(feature = "full", ts(export))] pub struct DeleteImageParams { pub filename: String, - pub token: String, } #[skip_serializing_none] @@ -43,7 +42,6 @@ pub struct ImageProxyParams { pub struct UploadImageResponse { pub image_url: Url, pub filename: String, - pub delete_token: String, } /// Parameter for setting community icon or banner. Can't use POST data here as it already contains diff --git a/crates/api_common/src/request.rs b/crates/api_common/src/request.rs index b0e37915b..a70a685ef 100644 --- a/crates/api_common/src/request.rs +++ b/crates/api_common/src/request.rs @@ -265,7 +265,6 @@ pub struct PictrsResponse { #[derive(Deserialize, Serialize, Debug)] pub struct PictrsFile { pub file: String, - pub delete_token: String, pub details: PictrsFileDetails, } @@ -355,24 +354,18 @@ pub async fn purge_image_from_pictrs(image_url: &Url, context: &LemmyContext) -> } } -pub async fn delete_image_from_pictrs( - alias: &str, - delete_token: &str, - context: &LemmyContext, -) -> LemmyResult<()> { +pub async fn delete_image_from_pictrs(alias: &str, context: &LemmyContext) -> LemmyResult<()> { // Delete db row if any (old Lemmy versions didnt generate this). LocalImage::delete_by_alias(&mut context.pool(), alias) .await .ok(); let pictrs_config = context.settings().pictrs()?; - let url = format!( - "{}image/delete/{}/{}", - pictrs_config.url, &delete_token, &alias - ); + let url = format!("{}internal/delete?alias={}", pictrs_config.url, &alias); context .pictrs_client() - .delete(&url) + .post(&url) + .header("X-Api-Token", pictrs_config.api_key.unwrap_or_default()) .timeout(REQWEST_TIMEOUT) .send() .await? @@ -421,7 +414,6 @@ async fn generate_pictrs_thumbnail(image_url: &Url, context: &LemmyContext) -> L // IE, a local user shouldn't get to delete the thumbnails for their link posts local_user_id: None, pictrs_alias: image.file.clone(), - pictrs_delete_token: image.delete_token.clone(), }; let protocol_and_hostname = context.settings().get_protocol_and_hostname(); let thumbnail_url = image.image_url(&protocol_and_hostname)?; diff --git a/crates/api_common/src/utils.rs b/crates/api_common/src/utils.rs index 885c766d0..e508e04da 100644 --- a/crates/api_common/src/utils.rs +++ b/crates/api_common/src/utils.rs @@ -680,13 +680,9 @@ async fn delete_local_user_images(person_id: PersonId, context: &LemmyContext) - // Delete their images for upload in pictrs_uploads { - delete_image_from_pictrs( - &upload.local_image.pictrs_alias, - &upload.local_image.pictrs_delete_token, - context, - ) - .await - .ok(); + delete_image_from_pictrs(&upload.local_image.pictrs_alias, context) + .await + .ok(); } } Ok(()) diff --git a/crates/db_schema/src/impls/images.rs b/crates/db_schema/src/impls/images.rs index 8ded98e41..df894f68d 100644 --- a/crates/db_schema/src/impls/images.rs +++ b/crates/db_schema/src/impls/images.rs @@ -1,5 +1,5 @@ use crate::{ - newtypes::DbUrl, + newtypes::{DbUrl, LocalUserId}, schema::{image_details, local_image, remote_image}, source::images::{ImageDetails, ImageDetailsForm, LocalImage, LocalImageForm, RemoteImage}, utils::{get_conn, DbPool}, @@ -9,6 +9,7 @@ use diesel::{ insert_into, result::Error, select, + BoolExpressionMethods, ExpressionMethods, NotFound, QueryDsl, @@ -47,6 +48,23 @@ impl LocalImage { .await } + pub async fn delete_by_alias_and_user( + pool: &mut DbPool<'_>, + alias: &str, + local_user_id: LocalUserId, + ) -> Result { + let conn = &mut get_conn(pool).await?; + diesel::delete( + local_image::table.filter( + local_image::pictrs_alias + .eq(alias) + .and(local_image::local_user_id.eq(local_user_id)), + ), + ) + .get_result(conn) + .await + } + pub async fn delete_by_url(pool: &mut DbPool<'_>, url: &DbUrl) -> Result { let alias = url.as_str().split('/').last().ok_or(NotFound)?; Self::delete_by_alias(pool, alias).await diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs index c4698f641..30e4550ef 100644 --- a/crates/db_schema/src/schema.rs +++ b/crates/db_schema/src/schema.rs @@ -386,7 +386,6 @@ diesel::table! { local_image (pictrs_alias) { local_user_id -> Nullable, pictrs_alias -> Text, - pictrs_delete_token -> Text, published -> Timestamptz, } } diff --git a/crates/db_schema/src/source/image_upload.rs b/crates/db_schema/src/source/image_upload.rs deleted file mode 100644 index db840dc1d..000000000 --- a/crates/db_schema/src/source/image_upload.rs +++ /dev/null @@ -1,39 +0,0 @@ -use crate::newtypes::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; - -#[skip_serializing_none] -#[derive(PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] -#[cfg_attr( - feature = "full", - derive(Queryable, Selectable, Associations, Identifiable, TS) -)] -#[cfg_attr(feature = "full", diesel(table_name = image_upload))] -#[cfg_attr(feature = "full", diesel(primary_key(pictrs_alias)))] -#[cfg_attr(feature = "full", ts(export))] -#[cfg_attr( - feature = "full", - diesel(belongs_to(crate::source::local_user::LocalUser)) -)] -#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))] -pub struct ImageUpload { - pub local_user_id: LocalUserId, - pub pictrs_alias: String, - pub pictrs_delete_token: String, - pub published: DateTime, -} - -#[derive(Debug, Clone)] -#[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/images.rs b/crates/db_schema/src/source/images.rs index acd339d8e..34d1bb43b 100644 --- a/crates/db_schema/src/source/images.rs +++ b/crates/db_schema/src/source/images.rs @@ -26,7 +26,6 @@ pub struct LocalImage { #[cfg_attr(feature = "full", ts(optional))] pub local_user_id: Option, pub pictrs_alias: String, - pub pictrs_delete_token: String, pub published: DateTime, } @@ -36,7 +35,6 @@ pub struct LocalImage { pub struct LocalImageForm { pub local_user_id: Option, pub pictrs_alias: String, - pub pictrs_delete_token: String, } /// Stores all images which are hosted on remote domains. When attempting to proxy an image, it diff --git a/crates/routes/src/images/delete.rs b/crates/routes/src/images/delete.rs index b28c87c6c..032147317 100644 --- a/crates/routes/src/images/delete.rs +++ b/crates/routes/src/images/delete.rs @@ -3,6 +3,7 @@ use actix_web::web::*; use lemmy_api_common::{ context::LemmyContext, image::{CommunityIdQuery, DeleteImageParams}, + request::delete_image_from_pictrs, utils::{is_admin, is_mod_or_admin}, SuccessResponse, }; @@ -121,27 +122,19 @@ pub async fn delete_user_banner( Ok(Json(SuccessResponse::default())) } -// TODO: get rid of delete tokens and allow deletion by admin or uploader pub async fn delete_image( data: Json, context: Data, - // require login - _local_user_view: LocalUserView, + local_user_view: LocalUserView, ) -> LemmyResult> { - let pictrs_config = context.settings().pictrs()?; - let url = format!( - "{}image/delete/{}/{}", - pictrs_config.url, &data.token, &data.filename - ); + LocalImage::delete_by_alias_and_user( + &mut context.pool(), + &data.filename, + local_user_view.local_user.id, + ) + .await?; - context - .pictrs_client() - .delete(url) - .send() - .await? - .error_for_status()?; - - LocalImage::delete_by_alias(&mut context.pool(), &data.filename).await?; + delete_image_from_pictrs(&data.filename, &context).await?; Ok(Json(SuccessResponse::default())) } diff --git a/crates/routes/src/images/upload.rs b/crates/routes/src/images/upload.rs index 50660419b..6ddc08458 100644 --- a/crates/routes/src/images/upload.rs +++ b/crates/routes/src/images/upload.rs @@ -215,7 +215,6 @@ pub async fn do_upload_image( let form = LocalImageForm { local_user_id: Some(local_user_view.local_user.id), pictrs_alias: image.file.to_string(), - pictrs_delete_token: image.delete_token.to_string(), }; let protocol_and_hostname = context.settings().get_protocol_and_hostname(); @@ -234,6 +233,5 @@ pub async fn do_upload_image( Ok(UploadImageResponse { image_url: url, filename: image.file, - delete_token: image.delete_token, }) } diff --git a/crates/routes/src/images/utils.rs b/crates/routes/src/images/utils.rs index 80108360c..c614d9ad3 100644 --- a/crates/routes/src/images/utils.rs +++ b/crates/routes/src/images/utils.rs @@ -104,7 +104,7 @@ pub(super) async fn delete_old_image( .await .ok(); if let Some(image) = image { - delete_image_from_pictrs(&image.pictrs_alias, &image.pictrs_delete_token, context).await?; + delete_image_from_pictrs(&image.pictrs_alias, context).await?; } } Ok(()) diff --git a/migrations/2025-01-09-144233_no-image-token/down.sql b/migrations/2025-01-09-144233_no-image-token/down.sql new file mode 100644 index 000000000..786862248 --- /dev/null +++ b/migrations/2025-01-09-144233_no-image-token/down.sql @@ -0,0 +1,19 @@ +ALTER TABLE local_image + ADD COLUMN pictrs_delete_token text NOT NULL DEFAULT ''; + +ALTER TABLE local_image + ALTER COLUMN pictrs_delete_token DROP DEFAULT; + +ALTER TABLE local_image + ADD COLUMN published_new timestamp with time zone DEFAULT now() NOT NULL; + +UPDATE + local_image +SET + published_new = published; + +ALTER TABLE local_image + DROP COLUMN published; + +ALTER TABLE local_image RENAME published_new TO published; + diff --git a/migrations/2025-01-09-144233_no-image-token/up.sql b/migrations/2025-01-09-144233_no-image-token/up.sql new file mode 100644 index 000000000..091815245 --- /dev/null +++ b/migrations/2025-01-09-144233_no-image-token/up.sql @@ -0,0 +1,3 @@ +ALTER TABLE local_image + DROP COLUMN pictrs_delete_token; +