From af9a4b18e7eea7d6545733564e1c273628dd4a84 Mon Sep 17 00:00:00 2001 From: Trezy Date: Tue, 07 Jul 2026 19:56:48 +0000 Subject: [PATCH] fix: ensure superadmin-created admin API keys respect scopes Signed-off-by: Trezy --- tests/e2e_admin.rs | 71 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/admin/auth.rs | 24 ++++++++++++++++++++++-- 2 file(s) changed, 93 insertion(s)(+), 2 deletion(s)(-) diff --git a/tests/e2e_admin.rs b/tests/e2e_admin.rs --- a/tests/e2e_admin.rs +++ b/tests/e2e_admin.rs @@ -57,6 +57,77 @@ .unwrap() } +async fn response_json(resp: axum::http::Response) -> Value { + let body = resp.into_body().collect().await.unwrap().to_bytes(); + serde_json::from_slice(&body).unwrap_or(json!(null)) +} + +fn bearer_get(uri: &str, key: &str) -> Request { + Request::builder() + .uri(uri) + .header("authorization", format!("Bearer {key}")) + .body(Body::empty()) + .unwrap() +} + +/// A super admin's scoped API key must be limited to its stored permissions — +/// it must NOT inherit the owner's super privileges (H5). +#[tokio::test] +#[serial] +async fn super_user_api_key_is_bounded_by_its_permissions() { + common::require_db!(); + let app = TestApp::new().await; + + // The TestApp admin is a super user. Create a key scoped to stats:read only. + let create = app + .router + .clone() + .oneshot(admin_post( + "/admin/api-keys", + app.admin_cookie(), + &json!({ "name": "ci-monitor", "permissions": ["stats:read"] }), + )) + .await + .unwrap(); + assert_eq!(create.status(), StatusCode::CREATED); + let key = response_json(create).await["key"] + .as_str() + .expect("api key returned") + .to_string(); + + // It can reach the permission it was granted. + let allowed = app + .router + .clone() + .oneshot(bearer_get("/admin/stats", &key)) + .await + .unwrap(); + assert_eq!(allowed.status(), StatusCode::OK); + + // It must NOT reach a permission it wasn't granted, even though its owner + // is super. Before the fix this returned 200 (full super via the key). + let denied = app + .router + .clone() + .oneshot(bearer_get("/admin/lexicons", &key)) + .await + .unwrap(); + assert_eq!( + denied.status(), + StatusCode::FORBIDDEN, + "a super user's scoped key must not grant permissions outside its list" + ); + + // And it must not reach super-only operations (user management). + let users = app + .router + .clone() + .oneshot(bearer_get("/admin/users", &key)) + .await + .unwrap(); + assert_ne!(users.status(), StatusCode::OK); +} + // --------------------------------------------------------------------------- // Auth tests // --------------------------------------------------------------------------- diff --git a/src/admin/auth.rs b/src/admin/auth.rs --- a/src/admin/auth.rs +++ b/src/admin/auth.rs @@ -254,8 +254,13 @@ let key_permissions: Vec = serde_json::from_str(&permissions_json).unwrap_or_default(); + // An API key is bounded by its own stored permission list and never + // carries super privileges — otherwise a super admin's "read-only" key + // would grant full admin (H5). A super user implicitly holds every + // permission, so their key's list is used as-is; a non-super user's key + // is additionally intersected with what that user actually holds. let permissions = if is_super { - HashSet::new() + parse_permissions(&key_permissions) } else { Self::load_api_key_permissions(&state.db, &user_id, &key_permissions, backend).await? }; @@ -277,10 +282,25 @@ Ok(Some(UserAuth { did, user_id, - is_super, + // A key is never super, regardless of its owner: `require()` must + // consult the key's permissions, and super-only operations (user + // management, transfer_super) stay unavailable via API keys. + is_super: false, permissions, db: state.db.clone(), db_backend: backend, })) } +} + +/// Parse a stored key permission list (JSON strings) into a permission set, +/// dropping any unrecognized entries. Used for super-user keys, whose owner +/// implicitly holds every permission so no intersection is needed. +fn parse_permissions(key_permissions: &[String]) -> HashSet { + key_permissions + .iter() + .filter_map(|s| { + serde_json::from_value::(serde_json::Value::String(s.clone())).ok() + }) + .collect() } -- tangled.sh