diff --git a/backend/Cargo.lock b/backend/Cargo.lock index f65dd37..4b39d05 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.2" +version = "0.87.3" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index b9ad188..caca5e2 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.2" +version = "0.87.3" edition = "2021" default-run = "mangalord" diff --git a/backend/src/api/mangas.rs b/backend/src/api/mangas.rs index d1befb2..10ea644 100644 --- a/backend/src/api/mangas.rs +++ b/backend/src/api/mangas.rs @@ -235,7 +235,7 @@ async fn update( if !repo::manga::exists(&state.db, id).await? { return Err(AppError::NotFound); } - require_can_edit(&state, id, user.id).await?; + require_can_edit(&state, id, user.id, user.is_admin).await?; if let Some(ref status) = patch.status { let trimmed = status.trim(); @@ -306,7 +306,7 @@ async fn put_cover( if !repo::manga::exists(&state.db, id).await? { return Err(AppError::NotFound); } - require_can_edit(&state, id, user.id).await?; + require_can_edit(&state, id, user.id, user.is_admin).await?; let mut cover: Option = None; while let Some(field) = next_field(&mut multipart).await? { @@ -354,7 +354,7 @@ async fn delete_cover( if !repo::manga::exists(&state.db, id).await? { return Err(AppError::NotFound); } - require_can_edit(&state, id, user.id).await?; + require_can_edit(&state, id, user.id, user.is_admin).await?; if let Some(key) = repo::manga::get(&state.db, id).await?.cover_image_path { match state.storage.delete(&key).await { Ok(()) | Err(StorageError::NotFound) => {} @@ -467,11 +467,16 @@ fn validate_new_manga(input: &NewManga) -> AppResult<()> { /// exist (the caller runs [`repo::manga::exists`] first so a missing id /// surfaces as `NotFound`, not `Forbidden`). /// -/// Rule: a non-NULL `uploaded_by` must match the current user. Legacy -/// rows with `uploaded_by IS NULL` (pre-migration-0011) are still -/// editable by any signed-in user — there's nobody to gate on yet, and -/// the historical-data note in 0011 acknowledges the gap. Once an -/// admin role lands the NULL case can flip to admin-only. +/// Rule: a non-NULL `uploaded_by` must match the current user, OR the +/// caller is an admin. Rows with `uploaded_by IS NULL` (crawler-imported +/// + legacy pre-0011) are admin-only. +/// +/// Why: every crawler row has NULL `uploaded_by` (see +/// `repo::crawler::upsert_manga`). The earlier "any signed-in user can +/// edit a NULL row" rule meant `register → PATCH /api/v1/mangas/` +/// rewrote the catalog for free, and `delete_cover` removed blobs from +/// `storage`. Now that the admin role exists (migration 0018), gate +/// crawler/legacy rows on it. /// /// Returns `Forbidden` (not `NotFound`) on owner mismatch — mangas /// are listable via `GET /mangas`, so existence isn't a secret and @@ -479,11 +484,18 @@ fn validate_new_manga(input: &NewManga) -> AppResult<()> { /// `repo::collection::require_owner`, which collapses both states to /// `NotFound` because collections are private to a user and existence /// itself is information worth hiding from non-owners. -async fn require_can_edit(state: &AppState, manga_id: Uuid, user_id: Uuid) -> AppResult<()> { +async fn require_can_edit( + state: &AppState, + manga_id: Uuid, + user_id: Uuid, + is_admin: bool, +) -> AppResult<()> { match repo::manga::uploaded_by(&state.db, manga_id).await? { - Some(owner) if owner != user_id => Err(AppError::Forbidden), - // Some(owner) == user_id (good) or None (legacy row, no owner). - _ => Ok(()), + Some(owner) if owner == user_id => Ok(()), + Some(_) if is_admin => Ok(()), + Some(_) => Err(AppError::Forbidden), + None if is_admin => Ok(()), + None => Err(AppError::Forbidden), } } diff --git a/backend/tests/api_mangas_metadata.rs b/backend/tests/api_mangas_metadata.rs index 107f9d2..13db87c 100644 --- a/backend/tests/api_mangas_metadata.rs +++ b/backend/tests/api_mangas_metadata.rs @@ -611,18 +611,18 @@ async fn patch_allowed_for_uploader(pool: PgPool) { assert_eq!(resp.status(), StatusCode::OK); } -/// Legacy rows with `uploaded_by IS NULL` (created before migration -/// 0011) remain editable by any signed-in user. Without this carve-out -/// the historical-data note in 0011 would be broken. +/// Rows with `uploaded_by IS NULL` — crawler-imported plus any pre-0011 +/// legacy rows — are admin-only. A regular user editing them used to be +/// allowed; that meant `register → PATCH /mangas/` rewrote the +/// catalog for free. #[sqlx::test(migrations = "./migrations")] -async fn patch_allowed_on_legacy_null_uploader(pool: PgPool) { +async fn patch_null_uploader_rejected_for_non_admin(pool: PgPool) { let h = common::harness(pool.clone()); let (_, cookie) = common::register_user(&h.app).await; - let created = create_manga(&h.app, &cookie, json!({ "title": "Legacy" })).await; + let created = create_manga(&h.app, &cookie, json!({ "title": "Catalog" })).await; let id = id_of(&created); - // Simulate a row uploaded before the column existed: clear - // uploaded_by directly via SQL. + // Simulate a crawler-imported / pre-0011 row: clear uploaded_by. sqlx::query("UPDATE mangas SET uploaded_by = NULL WHERE id = $1") .bind(id) .execute(&pool) @@ -639,5 +639,42 @@ async fn patch_allowed_on_legacy_null_uploader(pool: PgPool) { )) .await .unwrap(); + assert_eq!(resp.status(), StatusCode::FORBIDDEN); +} + +/// Admins CAN edit NULL-`uploaded_by` rows, since they're the operators +/// curating the crawled catalog. Mirror of the test above with the +/// caller promoted. +#[sqlx::test(migrations = "./migrations")] +async fn patch_null_uploader_allowed_for_admin(pool: PgPool) { + let h = common::harness(pool.clone()); + let (_, cookie) = common::register_user(&h.app).await; + let created = create_manga(&h.app, &cookie, json!({ "title": "Catalog" })).await; + let id = id_of(&created); + + sqlx::query("UPDATE mangas SET uploaded_by = NULL WHERE id = $1") + .bind(id) + .execute(&pool) + .await + .unwrap(); + + // Mint an admin and let them edit the row. + let (admin_name, admin_cookie) = common::register_user(&h.app).await; + let admin = mangalord::repo::user::find_by_username(&pool, &admin_name) + .await + .unwrap() + .unwrap(); + mangalord::repo::user::set_is_admin_unchecked(&pool, admin.id, true) + .await + .unwrap(); + let resp = h + .app + .oneshot(common::patch_json_with_cookie( + &format!("/api/v1/mangas/{id}"), + json!({ "status": "completed" }), + &admin_cookie, + )) + .await + .unwrap(); assert_eq!(resp.status(), StatusCode::OK); } diff --git a/frontend/package.json b/frontend/package.json index f9b4fa6..f06abe1 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.2", + "version": "0.87.3", "private": true, "type": "module", "scripts": {