Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pictrs delete token #5317

Merged
merged 59 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
56ab67f
Split image endpoints into API v3 and v4
Nutomic Dec 12, 2024
05843fb
Move into subfolders
Nutomic Dec 12, 2024
cfa866a
Upload avatar endpoint and other changes
Nutomic Dec 13, 2024
d252be2
Various other changes
Nutomic Dec 13, 2024
e815778
clippy
Nutomic Dec 13, 2024
60ba7af
config options
Nutomic Dec 13, 2024
7c771d2
fix ts bindings
Nutomic Dec 13, 2024
8ae4b40
fix api tests
Nutomic Dec 17, 2024
b0d4bdb
Add option to disable image upload (fixes #1118)
Nutomic Dec 17, 2024
55b8ace
split files into upload, download
Nutomic Dec 17, 2024
460ccc8
move sitemap to top level, not in api
Nutomic Dec 18, 2024
5f06195
simplify code
Nutomic Dec 18, 2024
5f49b2a
add upload user banner
Nutomic Dec 18, 2024
a6f7e76
community icon/banner
Nutomic Dec 18, 2024
ada8f1b
site icon/banner
Nutomic Dec 18, 2024
4951aa0
update js client
Nutomic Dec 18, 2024
2ec8cd3
Merge branch 'main' into image-api-rework
Nutomic Jan 3, 2025
961c7f1
wip
Nutomic Jan 8, 2025
705703f
add delete endpoints
Nutomic Jan 9, 2025
216fca2
change comment
Nutomic Jan 9, 2025
6f91754
Merge branch 'main' into image-api-rework
Nutomic Jan 9, 2025
4d51f53
optimization
Nutomic Jan 9, 2025
96cf017
move fn
Nutomic Jan 9, 2025
f04da5e
1024px banner
Nutomic Jan 9, 2025
7cbbb9a
dont use static client
Nutomic Jan 10, 2025
83c3304
fix api tests
Nutomic Jan 10, 2025
f040f91
shear
Nutomic Jan 10, 2025
6123659
proxy pictrs in request.rs (fixes #5270)
Nutomic Jan 10, 2025
ec74669
clippy
Nutomic Jan 10, 2025
f10d0f1
Get rid of pictrs delete token
Nutomic Jan 9, 2025
b36c16a
remove delete token params
Nutomic Jan 10, 2025
ee091c7
try to fix api tests
Nutomic Jan 10, 2025
44c83e8
fmt
Nutomic Jan 10, 2025
76f4508
skip api tests
Nutomic Jan 10, 2025
fa8aaba
clippy
Nutomic Jan 10, 2025
c9dfbfe
create user
Nutomic Jan 10, 2025
7f78275
debug
Nutomic Jan 10, 2025
2e7a961
dbg
Nutomic Jan 10, 2025
a1e7c1c
ignore test
Nutomic Jan 10, 2025
1c12993
test
Nutomic Jan 13, 2025
e484489
image
Nutomic Jan 13, 2025
a45fcc5
run another
Nutomic Jan 13, 2025
4b3f2f5
fixed?
Nutomic Jan 13, 2025
4b94043
Merge branch 'main' into image-api-rework
Nutomic Jan 13, 2025
91a67cc
Merge branch 'image-api-rework' into pictrs-delete-token
Nutomic Jan 13, 2025
79cdb83
clippy
Nutomic Jan 13, 2025
2be56b7
fix
Nutomic Jan 13, 2025
a665515
Merge branch 'image-api-rework' into pictrs-delete-token
Nutomic Jan 13, 2025
0692900
migration with column order
Nutomic Jan 13, 2025
95bd69b
drop default
Nutomic Jan 13, 2025
47638cc
fix health check
Nutomic Jan 13, 2025
01730ef
Merge branch 'image-api-rework' into pictrs-delete-token
Nutomic Jan 13, 2025
7aff084
Merge branch 'main' into pictrs-delete-token
Nutomic Jan 14, 2025
98a09c5
update client
Nutomic Jan 14, 2025
0e11b8c
remove unused
Nutomic Jan 14, 2025
1e9a5b7
fix
Nutomic Jan 14, 2025
6cfd825
reuse delete_image_from_pictrs
Nutomic Jan 15, 2025
61bf184
Merge branch 'main' into pictrs-delete-token
Nutomic Jan 15, 2025
9d592d6
update lib
Nutomic Jan 15, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api_tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 5 additions & 5 deletions api_tests/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions api_tests/src/image.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? "");
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion api_tests/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 0 additions & 2 deletions crates/api_common/src/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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
Expand Down
16 changes: 4 additions & 12 deletions crates/api_common/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ pub struct PictrsResponse {
#[derive(Deserialize, Serialize, Debug)]
pub struct PictrsFile {
pub file: String,
pub delete_token: String,
pub details: PictrsFileDetails,
}

Expand Down Expand Up @@ -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?
Expand Down Expand Up @@ -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)?;
Expand Down
10 changes: 3 additions & 7 deletions crates/api_common/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
20 changes: 19 additions & 1 deletion crates/db_schema/src/impls/images.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand All @@ -9,6 +9,7 @@ use diesel::{
insert_into,
result::Error,
select,
BoolExpressionMethods,
ExpressionMethods,
NotFound,
QueryDsl,
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion crates/db_schema/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@ diesel::table! {
local_image (pictrs_alias) {
local_user_id -> Nullable<Int4>,
pictrs_alias -> Text,
pictrs_delete_token -> Text,
published -> Timestamptz,
}
}
Expand Down
39 changes: 0 additions & 39 deletions crates/db_schema/src/source/image_upload.rs

This file was deleted.

2 changes: 0 additions & 2 deletions crates/db_schema/src/source/images.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}

Expand All @@ -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
Expand Down
27 changes: 10 additions & 17 deletions crates/routes/src/images/delete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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()))
}
2 changes: 0 additions & 2 deletions crates/routes/src/images/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -234,6 +233,5 @@ pub async fn do_upload_image(
Ok(UploadImageResponse {
image_url: url,
filename: image.file,
delete_token: image.delete_token,
})
}
2 changes: 1 addition & 1 deletion crates/routes/src/images/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
19 changes: 19 additions & 0 deletions migrations/2025-01-09-144233_no-image-token/down.sql
Original file line number Diff line number Diff line change
@@ -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;
dessalines marked this conversation as resolved.
Show resolved Hide resolved

3 changes: 3 additions & 0 deletions migrations/2025-01-09-144233_no-image-token/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE local_image
DROP COLUMN pictrs_delete_token;