test(authz): bearer-authed admin cannot edit/cover NULL-uploader mangas (0.87.19)
0.87.10 followup from the rereview. The fix lacked a test for the specific axis it changed (admin authority via cookie only). New test mints a bearer for a promoted admin and asserts PATCH/PUT-cover/DELETE- cover all 403 on NULL-uploader rows. Mutation-confirmed. Also: `MultipartBuilder::finalize` now `pub` for bearer-multipart use; `admin_csrf_guard` rustdoc updated to describe the post-0.87.10 cookie-precedence rule. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.87.18"
|
||||
version = "0.87.19"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.87.18"
|
||||
version = "0.87.19"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -1058,9 +1058,17 @@ const ADMIN_PATH_PREFIX: &str = "/api/v1/admin/";
|
||||
/// the configured allowlist. Safe methods (`GET`/`HEAD`/`OPTIONS`) are
|
||||
/// always allowed.
|
||||
///
|
||||
/// **Bearer-token requests** (`Authorization: Bearer …`) are bot API
|
||||
/// callers — they can't be a CSRF vector because the browser never
|
||||
/// attaches that header automatically. Skip the check for them.
|
||||
/// **Bearer-token-only requests** (`Authorization: Bearer …` with NO
|
||||
/// session cookie) are bot API callers — they can't be a CSRF vector
|
||||
/// because the browser never attaches the Authorization header
|
||||
/// automatically. Skip the check for them.
|
||||
///
|
||||
/// **Bearer + cookie (the "cookie-ride")** is treated as cookie-auth.
|
||||
/// An attacker page can mint `Authorization: Bearer junk` on a
|
||||
/// credentialed cross-site POST; the cookie carries the actual
|
||||
/// authority. Closing this hole means cookie precedence: as soon as a
|
||||
/// session cookie is present, the CSRF gate fires regardless of the
|
||||
/// Authorization header. (0.87.10 closed a 0.87.2 regression here.)
|
||||
///
|
||||
/// **Cookie-auth requests** must:
|
||||
/// * be from an allowed origin (`Origin` then `Referer`), AND
|
||||
@@ -1073,7 +1081,11 @@ const ADMIN_PATH_PREFIX: &str = "/api/v1/admin/";
|
||||
/// — this used to silently let everything through, so an operator who
|
||||
/// forgot to set `ADMIN_ALLOWED_ORIGINS` shipped an unguarded admin
|
||||
/// surface. Operators on a pure-bot-token deploy aren't impacted (their
|
||||
/// requests carry `Authorization: Bearer …` and skip the gate above).
|
||||
/// requests carry only `Authorization: Bearer …` with no session cookie
|
||||
/// and skip the gate via the bearer-only branch above).
|
||||
///
|
||||
/// **No auth at all** (no cookie, no bearer): bypass the CSRF gate so
|
||||
/// the auth extractor returns a clean 401 instead of a confusing 403.
|
||||
async fn admin_csrf_guard(
|
||||
State(state): State<AppState>,
|
||||
req: Request,
|
||||
|
||||
@@ -757,3 +757,125 @@ async fn patch_null_uploader_allowed_for_admin(pool: PgPool) {
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), StatusCode::OK);
|
||||
}
|
||||
|
||||
/// 0.87.10 split `require_can_edit` so admin authority must come via a
|
||||
/// session cookie — bearer tokens (bot callers) DO NOT carry admin
|
||||
/// powers per the auth-module contract. A bearer-authed admin trying
|
||||
/// to PATCH/PUT-cover/DELETE-cover a NULL-`uploaded_by` row must be
|
||||
/// rejected. Mechanical revert of `admin_via_session(&session)` to
|
||||
/// `user.is_admin` would let bearer-authed admins through and pass
|
||||
/// the 0.87.10 cookie tests; this is the missing axis the rereview
|
||||
/// asked for.
|
||||
#[sqlx::test(migrations = "./migrations")]
|
||||
async fn bearer_authed_admin_cannot_edit_null_uploader(pool: PgPool) {
|
||||
let h = common::harness(pool.clone());
|
||||
|
||||
// Catalog row with NULL uploaded_by (the crawler shape).
|
||||
let (_, owner_cookie) = common::register_user(&h.app).await;
|
||||
let created = create_manga(&h.app, &owner_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();
|
||||
|
||||
// Register a user, promote to admin, mint a bearer token from
|
||||
// their session.
|
||||
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 mint_resp = h
|
||||
.app
|
||||
.clone()
|
||||
.oneshot(common::post_json_with_cookie(
|
||||
"/api/v1/auth/tokens",
|
||||
json!({ "name": "ci-bot" }),
|
||||
&admin_cookie,
|
||||
))
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(mint_resp.status(), StatusCode::CREATED);
|
||||
let bearer = common::body_json(mint_resp).await["bearer"]
|
||||
.as_str()
|
||||
.unwrap()
|
||||
.to_string();
|
||||
|
||||
// PATCH via bearer — must 403 even though the user behind the
|
||||
// token IS an admin. CurrentUser admits the call, but
|
||||
// `admin_via_session` reads off `Option<CurrentSessionUser>`
|
||||
// which is None on a bearer request.
|
||||
let patch_resp = h
|
||||
.app
|
||||
.clone()
|
||||
.oneshot({
|
||||
let body = json!({ "status": "completed" });
|
||||
axum::http::Request::builder()
|
||||
.method("PATCH")
|
||||
.uri(format!("/api/v1/mangas/{id}"))
|
||||
.header(axum::http::header::CONTENT_TYPE, "application/json")
|
||||
.header(axum::http::header::AUTHORIZATION, format!("Bearer {bearer}"))
|
||||
.body(axum::body::Body::from(body.to_string()))
|
||||
.unwrap()
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
patch_resp.status(),
|
||||
StatusCode::FORBIDDEN,
|
||||
"bearer-authed admin must NOT be able to PATCH a NULL-uploader manga"
|
||||
);
|
||||
|
||||
// PUT cover via bearer — same gate, same answer.
|
||||
let png = common::fake_png_bytes();
|
||||
let (boundary, body) = common::MultipartBuilder::new()
|
||||
.add_file("cover", "cover.png", "image/png", &png)
|
||||
.finalize();
|
||||
let put_resp = h
|
||||
.app
|
||||
.clone()
|
||||
.oneshot({
|
||||
axum::http::Request::builder()
|
||||
.method("PUT")
|
||||
.uri(format!("/api/v1/mangas/{id}/cover"))
|
||||
.header(
|
||||
axum::http::header::CONTENT_TYPE,
|
||||
format!("multipart/form-data; boundary={boundary}"),
|
||||
)
|
||||
.header(axum::http::header::AUTHORIZATION, format!("Bearer {bearer}"))
|
||||
.body(axum::body::Body::from(body))
|
||||
.unwrap()
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
put_resp.status(),
|
||||
StatusCode::FORBIDDEN,
|
||||
"bearer-authed admin must NOT be able to put_cover on a NULL-uploader manga"
|
||||
);
|
||||
|
||||
// DELETE cover via bearer.
|
||||
let delete_resp = h
|
||||
.app
|
||||
.oneshot(
|
||||
axum::http::Request::builder()
|
||||
.method("DELETE")
|
||||
.uri(format!("/api/v1/mangas/{id}/cover"))
|
||||
.header(axum::http::header::AUTHORIZATION, format!("Bearer {bearer}"))
|
||||
.body(axum::body::Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
delete_resp.status(),
|
||||
StatusCode::FORBIDDEN,
|
||||
"bearer-authed admin must NOT be able to delete_cover on a NULL-uploader manga"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -615,7 +615,11 @@ impl MultipartBuilder {
|
||||
self.body.extend(b"\r\n");
|
||||
}
|
||||
|
||||
fn finalize(self) -> (String, Vec<u8>) {
|
||||
/// Pub so a test can mint a multipart body for a bearer-auth Request
|
||||
/// without going through the cookie helpers. Returns
|
||||
/// `(boundary, body)` ready to attach as `Content-Type:
|
||||
/// multipart/form-data; boundary={boundary}` and the request body.
|
||||
pub fn finalize(self) -> (String, Vec<u8>) {
|
||||
let mut body = self.body;
|
||||
body.extend(format!("--{}--\r\n", self.boundary).as_bytes());
|
||||
(self.boundary, body)
|
||||
|
||||
Reference in New Issue
Block a user