diff --git a/backend/accounts-service/src/accounts/repo.rs b/backend/accounts-service/src/accounts/repo.rs index adb4e8dd..0249bbd1 100644 --- a/backend/accounts-service/src/accounts/repo.rs +++ b/backend/accounts-service/src/accounts/repo.rs @@ -61,6 +61,17 @@ pub async fn set_avatar(pool: &PgPool, account_id: Uuid, s3_key: &str) -> Result Ok(()) } +/// Deletes the `avatars` row for one account. Returns `true` when a row existed +/// (so the caller can decide whether to also delete the S3 object) and `false` +/// when the user simply had no avatar — the API still treats that as success. +pub async fn clear_avatar(pool: &PgPool, account_id: Uuid) -> Result { + let result = sqlx::query("DELETE FROM avatars WHERE account_id = $1") + .bind(account_id) + .execute(pool) + .await?; + Ok(result.rows_affected() > 0) +} + /// Avatar reference for one account: its storage key plus an `uploaded_at` /// version (epoch seconds) used to cache-bust the served image URL. pub async fn avatar_ref( diff --git a/backend/accounts-service/src/avatars/handlers.rs b/backend/accounts-service/src/avatars/handlers.rs index 8c783f42..53a9d95b 100644 --- a/backend/accounts-service/src/avatars/handlers.rs +++ b/backend/accounts-service/src/avatars/handlers.rs @@ -70,6 +70,28 @@ pub async fn upload( })) } +/// DELETE /avatars — clears the caller's avatar. Removes the DB row and, if one +/// existed, the S3 object. Always returns 204, including when the user had no +/// avatar: the operation is idempotent, matching how the frontend's "Remove +/// avatar" button should behave after a successful click. +pub async fn delete( + State(state): State, + identity: GatewayIdentity, +) -> Result { + let account_id = identity.account_id; + let existed = repo::clear_avatar(&state.pool, account_id).await?; + if existed { + let key = format!("avatars/{account_id}.png"); + // S3 delete is best-effort: if MinIO is unreachable we already committed + // the DB row removal, and the object becomes orphaned but not served (the + // serve handler looks up keys via the DB, not directly). + if let Err(err) = state.storage.delete(&key).await { + tracing::warn!("avatar S3 delete failed for {account_id}: {err}"); + } + } + Ok(StatusCode::NO_CONTENT.into_response()) +} + /// Only serve the keys this service writes: `avatars/.png`. /// Anything else (traversal, other buckets/paths) is a 404. The gateway blocks /// dot-segments upstream, but the service validates independently. diff --git a/backend/accounts-service/src/avatars/storage.rs b/backend/accounts-service/src/avatars/storage.rs index 1a33bc57..3a0644e9 100644 --- a/backend/accounts-service/src/avatars/storage.rs +++ b/backend/accounts-service/src/avatars/storage.rs @@ -51,4 +51,17 @@ impl S3Storage { let aggregated = out.body.collect().await?; Ok(aggregated.into_bytes().to_vec()) } + + /// Deletes the object at `key`. MinIO's DeleteObject is idempotent — deleting + /// a missing key returns Ok. Callers on the app layer are expected to run + /// this only after a matching DB row existed, so a stray 404 here is silent. + pub async fn delete(&self, key: &str) -> anyhow::Result<()> { + self.client + .delete_object() + .bucket(&self.bucket) + .key(key) + .send() + .await?; + Ok(()) + } } diff --git a/backend/accounts-service/src/lib.rs b/backend/accounts-service/src/lib.rs index d020ba24..2d12f82d 100644 --- a/backend/accounts-service/src/lib.rs +++ b/backend/accounts-service/src/lib.rs @@ -89,6 +89,7 @@ pub fn build_app_with_mail(pool: sqlx::PgPool, cfg: &Config, mail: auth::reset:: let avatar_routes = Router::new() .route("/avatars", post(avatars::handlers::upload)) + .route("/avatars", axum::routing::delete(avatars::handlers::delete)) // Public avatar read served back through the gateway; the `{*key}` // wildcard is the storage key (`avatars/.png`). .route("/media/{*key}", get(avatars::handlers::serve)) diff --git a/backend/accounts-service/tests/avatar_upload.rs b/backend/accounts-service/tests/avatar_upload.rs index 85291d70..215584cb 100644 --- a/backend/accounts-service/tests/avatar_upload.rs +++ b/backend/accounts-service/tests/avatar_upload.rs @@ -121,3 +121,78 @@ async fn upload_without_identity_is_401() { .await .assert_status(StatusCode::UNAUTHORIZED); } + +#[tokio::test] +async fn delete_removes_the_row_and_returns_204() { + common::init_test_logging(); + common::ensure_avatar_bucket().await; + let pool = common::test_pool().await; + let server = common::test_server(accounts_service::build_app( + pool.clone(), + &common::test_config(), + )); + let (account_id, email) = common::register_account(&server).await; + + server + .post("/avatars") + .add_header(ACCOUNT_ID_HEADER, account_id.to_string()) + .multipart(form(png_bytes())) + .await + .assert_status_ok(); + + server + .delete("/avatars") + .add_header(ACCOUNT_ID_HEADER, account_id.to_string()) + .await + .assert_status(StatusCode::NO_CONTENT); + + let count: i64 = sqlx::query_as::<_, (i64,)>("SELECT COUNT(*) FROM avatars WHERE account_id = $1") + .bind(account_id) + .fetch_one(&pool) + .await + .unwrap() + .0; + assert_eq!(count, 0); + + sqlx::query("DELETE FROM accounts WHERE lower(email) = $1") + .bind(email.to_lowercase()) + .execute(&pool) + .await + .unwrap(); +} + +#[tokio::test] +async fn delete_without_avatar_is_idempotent() { + let pool = common::test_pool().await; + let server = common::test_server(accounts_service::build_app( + pool.clone(), + &common::test_config(), + )); + let (account_id, email) = common::register_account(&server).await; + + server + .delete("/avatars") + .add_header(ACCOUNT_ID_HEADER, account_id.to_string()) + .await + .assert_status(StatusCode::NO_CONTENT); + + sqlx::query("DELETE FROM accounts WHERE lower(email) = $1") + .bind(email.to_lowercase()) + .execute(&pool) + .await + .unwrap(); +} + +#[tokio::test] +async fn delete_without_identity_is_401() { + let pool = common::test_pool().await; + let server = common::test_server(accounts_service::build_app( + pool.clone(), + &common::test_config(), + )); + + server + .delete("/avatars") + .await + .assert_status(StatusCode::UNAUTHORIZED); +}