Repository: LemmyNet/lemmy (language: rust)

Issue:
[Bug]: Internal pictrs request tries to connect via external proxy
### Requirements

- [X] Is this a bug report? For questions or discussions use https://lemmy.ml/c/lemmy_support
- [X] Did you check to see if this issue already exists?
- [X] Is this only a single bug? Do not put multiple bugs in one issue.
- [X] Do you agree to follow the rules in our [Code of Conduct](https://join-lemmy.org/docs/code_of_conduct.html)?
- [X] Is this a backend issue? Use the [lemmy-ui](https://github.com/LemmyNet/lemmy-ui) repo for UI / frontend issues.

### Summary

https://github.com/LemmyNet/lemmy/commit/91848005828fd1a010f2fc06bb108eea1353b31f avoids proxying pictrs requests, useful when running in docker (see https://github.com/LemmyNet/lemmy/issues/3489). However, some pictrs requests are still attempted through the proxy from here:
https://github.com/LemmyNet/lemmy/blob/d346890b1f3eda453833ceba67f4c685e4f6e162/crates/api_common/src/request.rs#L376-L391

Currently, the requests fail when running in docker and using the system proxy. It should be set to not use the proxy like the other pictrs requests.

### Steps to Reproduce

1. Run lemmy in docker with proxy environment variables set
2. Observe attempts like the following being passed to the proxy: `TCP_MISS/502 4070 GET http://pictrs:8080/image/download?`

### Technical Details

n/a

### Version

BE: 0.19.7

### Lemmy Instance URL

_No response_


Discussion / hints:

Fix patch:
diff --git a/crates/api_common/src/image.rs b/crates/api_common/src/image.rs
index e93b367f60..9b3e4b00c0 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 b0e37915b2..a70a685ef6 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 885c766d09..e508e04dae 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 8ded98e414..df894f68d2 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<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
diff --git a/crates/db_schema/src/schema.rs b/crates/db_schema/src/schema.rs
index c4698f6419..30e4550eff 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<Int4>,
         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 db840dc1d3..0000000000
--- 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<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,
-}
diff --git a/crates/db_schema/src/source/images.rs b/crates/db_schema/src/source/images.rs
index acd339d8ea..34d1bb43bf 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<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
diff --git a/crates/routes/src/images/delete.rs b/crates/routes/src/images/delete.rs
index b28c87c6c2..0321473174 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<DeleteImageParams>,
   context: Data<LemmyContext>,
-  // require login
-  _local_user_view: LocalUserView,
+  local_user_view: LocalUserView,
 ) -> LemmyResult<Json<SuccessResponse>> {
-  let pictrs_config = context.settings().pictrs()?;
-  let url = format!(
-    "{}image/delete/{}/{}",
-    pictrs_config.url, &data.token, &data.filename
-  );
-
-  context
-    .pictrs_client()
-    .delete(url)
-    .send()
-    .await?
-    .error_for_status()?;
-
-  LocalImage::delete_by_alias(&mut context.pool(), &data.filename).await?;
+  LocalImage::delete_by_alias_and_user(
+    &mut context.pool(),
+    &data.filename,
+    local_user_view.local_user.id,
+  )
+  .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 50660419b1..6ddc084586 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 80108360ca..c614d9ad3b 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 0000000000..786862248d
--- /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 0000000000..091815245a
--- /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;
+


Test patch:
diff --git a/api_tests/package.json b/api_tests/package.json
index 8bed8c8c24..7e1fd8127e 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 71c88734f6..1a68b27772 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 1a686879b3..b5cc72fe0a 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 771b1dc573..6c4b4eba05 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;