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 <dull.bananas0@gmail.com>

* 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 <dull.bananas0@gmail.com>

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",

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
+        specifier: 0.20.0-no-delete-token.2
-        version: 0.20.0-modlog-combined.0
+        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:
+  lemmy-js-client@0.20.0-no-delete-token.2:
-    resolution: {integrity: sha512-lb3na39klOSE184hJJObMufKjHtm3Mrk42RHqyVNCYZQ+FAAbQzBFTuYyqv8QJV5TJlMmyFO2v1v/9cH72nLRg==}
+    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: {}
 

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

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;

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

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(
+pub async fn delete_image_from_pictrs(alias: &str, context: &LemmyContext) -> LemmyResult<()> {
-  alias: &str,
-  delete_token: &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!(
+  let url = format!("{}internal/delete?alias={}", pictrs_config.url, &alias);
-    "{}image/delete/{}/{}",
-    pictrs_config.url, &delete_token, &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)?;

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(
+      delete_image_from_pictrs(&upload.local_image.pictrs_alias, context)
-        &upload.local_image.pictrs_alias,
+        .await
-        &upload.local_image.pictrs_delete_token,
+        .ok();
-        context,
-      )
-      .await
-      .ok();
     }
   }
   Ok(())

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<Self, Error> {
+    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<Self, Error> {
     let alias = url.as_str().split('/').last().ok_or(NotFound)?;
     Self::delete_by_alias(pool, alias).await

crates/db_schema/src/schema.rs

@@ -386,7 +386,6 @@ diesel::table! {
     local_image (pictrs_alias) {
         local_user_id -> Nullable<Int4>,
         pictrs_alias -> Text,
-        pictrs_delete_token -> Text,
         published -> Timestamptz,
     }
 }

crates/db_schema/src/source/image_upload.rs

@@ -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<Utc>,
-}
-
-#[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,
-}

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<LocalUserId>,
   pub pictrs_alias: String,
-  pub pictrs_delete_token: String,
   pub published: DateTime<Utc>,
 }
 
@@ -36,7 +35,6 @@ pub struct LocalImage {
 pub struct LocalImageForm {
   pub local_user_id: Option<LocalUserId>,
   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

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<DeleteImageParams>,
   context: Data<LemmyContext>,
-  // require login
+  local_user_view: LocalUserView,
-  _local_user_view: LocalUserView,
 ) -> LemmyResult<Json<SuccessResponse>> {
-  let pictrs_config = context.settings().pictrs()?;
+  LocalImage::delete_by_alias_and_user(
-  let url = format!(
+    &mut context.pool(),
-    "{}image/delete/{}/{}",
+    &data.filename,
-    pictrs_config.url, &data.token, &data.filename
+    local_user_view.local_user.id,
-  );
+  )
+  .await?;
 
-  context
+  delete_image_from_pictrs(&data.filename, &context).await?;
-    .pictrs_client()
-    .delete(url)
-    .send()
-    .await?
-    .error_for_status()?;
-
-  LocalImage::delete_by_alias(&mut context.pool(), &data.filename).await?;
 
   Ok(Json(SuccessResponse::default()))
 }

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,
   })
 }

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(())

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;
+

migrations/2025-01-09-144233_no-image-token/up.sql

@@ -0,0 +1,3 @@
+ALTER TABLE local_image
+    DROP COLUMN pictrs_delete_token;
+