From f1f59ea1659aea455470ed652db92f5acd73e09a Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 25 Sep 2026 00:04:44 +0200 Subject: [PATCH] swarm-controller: make an existing forge user a site admin POST /api/forge/users/{name}/admin reads the account and, when it is not already a site admin, sets `admin` with admin_edit_user. It never creates one: a human's account is made by their first authelia login, so a missing one answers 404, saying the user has not logged in via SSO yet. An existing admin is a success with nothing sent. The edit carries `admin` alone. repo_creation_lockdown's login_name + source_id = 0 would turn an SSO-made account into a local one: in Forgejo 16 a source_id sets the login type. An agent's name is refused, and so is any name when the roster can't be read: a site admin ignores max_repo_creation, the lockdown that keeps an agent's token from creating a repo and self-merging in it. Refs #3782 --- swarm-controller/src/forge.rs | 75 ++++++++ swarm-controller/src/forge/agent_token.rs | 2 +- swarm-controller/src/forge/site_admin.rs | 128 +++++++++++++ swarm-controller/src/main.rs | 207 +++++++++++++++++++++- 4 files changed, 407 insertions(+), 5 deletions(-) create mode 100644 swarm-controller/src/forge/site_admin.rs diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index ec4cf3d9..e6f7a544 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -34,6 +34,7 @@ use utoipa::ToSchema; use crate::webhook::DeliveryKind; pub mod agent_token; +pub mod site_admin; /// An agent's open config-PR, as [`Client::list_open_config_prs`] reports it /// and `GET /api/agents/{name}/config-pr` serves it. @@ -1659,4 +1660,78 @@ mod tests { let body = lockdown_patch(&seen, "alice").expect("no lockdown PATCH sent"); assert_eq!(body["max_repo_creation"], 0); } + + /// Every `PATCH` body the stub saw. + fn patches(seen: &Seen) -> Vec { + seen.lock() + .unwrap() + .iter() + .filter(|(method, _, _)| method == "PATCH") + .map(|(_, _, body)| body.clone()) + .collect() + } + + /// Only `admin` goes out: a `source_id` would reset an SSO-made + /// account's login type, and the other fields would overwrite the user's + /// own settings. + #[tokio::test] + async fn make_site_admin_sets_admin_and_nothing_else() { + let (client, seen) = stub_forge( + (404, "{}"), + (200, r#"{"login":"mara","is_admin":false}"#), + (200, "{}"), + ) + .await; + + let plan = client.make_site_admin("mara").await.unwrap(); + + assert_eq!(plan, site_admin::Plan::Promote); + let bodies = patches(&seen); + assert_eq!(bodies.len(), 1, "{bodies:?}"); + let set: Vec<(&String, &serde_json::Value)> = bodies[0] + .as_object() + .unwrap() + .iter() + .filter(|(_, v)| !v.is_null()) + .collect(); + assert_eq!(set, [(&"admin".to_owned(), &serde_json::Value::Bool(true))]); + } + + #[tokio::test] + async fn make_site_admin_leaves_an_admin_alone() { + let (client, seen) = stub_forge( + (404, "{}"), + (200, r#"{"login":"mara","is_admin":true}"#), + (200, "{}"), + ) + .await; + + let plan = client.make_site_admin("mara").await.unwrap(); + + assert_eq!(plan, site_admin::Plan::AlreadyAdmin); + assert!(patches(&seen).is_empty()); + } + + /// A user who has not logged in yet is reported, not created: nothing is + /// sent but the read. + #[tokio::test] + async fn make_site_admin_does_not_create_a_missing_user() { + let (client, seen) = stub_forge( + (201, "{}"), + (404, r#"{"message":"user does not exist"}"#), + (200, "{}"), + ) + .await; + + let plan = client.make_site_admin("mara").await.unwrap(); + + assert_eq!(plan, site_admin::Plan::NoSuchUser); + let methods: Vec = seen + .lock() + .unwrap() + .iter() + .map(|(m, _, _)| m.clone()) + .collect(); + assert_eq!(methods, ["GET"]); + } } diff --git a/swarm-controller/src/forge/agent_token.rs b/swarm-controller/src/forge/agent_token.rs index caca0cac..5932bce8 100644 --- a/swarm-controller/src/forge/agent_token.rs +++ b/swarm-controller/src/forge/agent_token.rs @@ -153,7 +153,7 @@ pub fn plan(observed: &[(String, Observed)]) -> Vec { } /// Whether a forge error is a 404, whichever of its two shapes it came in. -fn is_not_found(e: &ForgejoError) -> bool { +pub(super) fn is_not_found(e: &ForgejoError) -> bool { match e { ForgejoError::ApiError(api) => { matches!(api.error_kind(), ApiErrorKind::NotFound { .. }) diff --git a/swarm-controller/src/forge/site_admin.rs b/swarm-controller/src/forge/site_admin.rs new file mode 100644 index 00000000..7d4de581 --- /dev/null +++ b/swarm-controller/src/forge/site_admin.rs @@ -0,0 +1,128 @@ +//! Making an existing human forge account a site admin: the whole job of +//! `POST /api/forge/users/{name}/admin`, which `swarmctl forge make-admin` +//! calls. +//! +//! Never creates the account. A human's account is made by their first +//! authelia login to the forge (`oauth2_client` in +//! `nix/host-modules/hive-forge/default.nix`), so a missing one means that +//! login has not happened yet, and making it here would be a second way in. +//! +//! The decision is pure ([`plan`]), so the tests pin it; the IO on either side +//! only reads or acts. Same split as [`super::agent_token`]. + +use anyhow::{Context, Result}; +use forgejo_api::structs::{EditUserOption, User}; + +use super::Client; +use super::agent_token::is_not_found; + +/// What one request does about the account. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Plan { + /// The forge has no user by this name: its owner has not logged in yet. + NoSuchUser, + /// Already a site admin: nothing to send. + AlreadyAdmin, + /// An ordinary account: make it a site admin. + Promote, +} + +/// Decide from the account as the forge reports it (`None` when there is no +/// such user). +pub fn plan(user: Option<&User>) -> Plan { + match user { + None => Plan::NoSuchUser, + Some(u) if u.is_admin == Some(true) => Plan::AlreadyAdmin, + Some(_) => Plan::Promote, + } +} + +/// The `admin_edit_user` body that sets `admin` and nothing else. +/// +/// Unlike [`super::repo_creation_lockdown`], no `login_name` + `source_id`: +/// in Forgejo 16 both are optional, and a `source_id` sets the account's login +/// type (`services/user/update.go`, `UpdateAuth`). `source_id = 0` would turn +/// an SSO-made account into a local one. +fn admin_edit() -> EditUserOption { + EditUserOption { + active: None, + admin: Some(true), + allow_create_organization: None, + allow_git_hook: None, + allow_import_local: None, + description: None, + email: None, + full_name: None, + hide_email: None, + location: None, + login_name: None, + max_repo_creation: None, + must_change_password: None, + password: None, + prohibit_login: None, + pronouns: None, + restricted: None, + source_id: None, + visibility: None, + website: None, + } +} + +impl Client { + /// Make the existing account `name` a site admin, and say what that took. + /// The caller has already refused an agent's name. + /// + /// # Errors + /// When the forge refuses the read or the edit. + pub async fn make_site_admin(&self, name: &str) -> Result { + let user = match self.api.user_get(name).await { + Ok(user) => Some(user), + Err(e) if is_not_found(&e) => None, + Err(e) => return Err(e).with_context(|| format!("read forge user {name}")), + }; + let plan = plan(user.as_ref()); + if plan == Plan::Promote { + self.api + .admin_edit_user(name, admin_edit()) + .await + .with_context(|| format!("make forge user {name} a site admin"))?; + tracing::info!(%name, "swarm forge: made the user a site admin"); + } + Ok(plan) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn user(is_admin: Option) -> User { + serde_json::from_value(serde_json::json!({ + "login": "mara", + "is_admin": is_admin, + })) + .expect("a user decodes") + } + + #[test] + fn a_missing_user_is_not_created() { + assert_eq!(plan(None), Plan::NoSuchUser); + } + + #[test] + fn an_admin_is_left_alone() { + assert_eq!(plan(Some(&user(Some(true)))), Plan::AlreadyAdmin); + } + + #[test] + fn an_ordinary_user_is_promoted() { + assert_eq!(plan(Some(&user(Some(false)))), Plan::Promote); + } + + /// No flag in the answer: the edit is sent. On an admin it is a no-op; + /// skipping it would report success for an ordinary account. + #[test] + fn an_unreported_admin_flag_is_promoted() { + assert_eq!(plan(Some(&user(None))), Plan::Promote); + } +} diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index e43fcb0e..3af90807 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -525,6 +525,7 @@ fn socket_path() -> PathBuf { (name = "agents", description = "creating agent identities at swarm level"), (name = "webhook", description = "swarm-wide forge webhook receipt"), (name = "repos", description = "forge repo browsing (swarm-ui's issue-report page)"), + (name = "forge", description = "forge site admins (`swarmctl forge make-admin`)"), ) )] struct ApiDoc; @@ -599,7 +600,8 @@ struct AppState { /// it only for a name that breaks a naming rule, to tell a new agent /// from an existing one, and warns when it cannot (`name_verdict`). **GET /// has nowhere to defer to** — there is no job, only an answer it - /// either has or does not. + /// either has or does not. `POST /api/forge/users/{name}/admin` reads + /// it for every name and refuses when it cannot (`admin_unless_agent`). auth: Option>, /// HMAC secret for swarm-wide forge webhooks, loaded once at startup. /// `None` when it could not be read or created — the webhook endpoint @@ -626,7 +628,7 @@ struct AppState { swarm_name: Option>, /// The forge client itself, for the read-only repo/issue-report /// routes (`GET /api/repos`, `GET /api/repos/{org}/{repo}/issue-report`) - /// — distinct from `config_prs`, which holds a *cache* built off this + /// and `POST /api/forge/users/{name}/admin` — distinct from `config_prs`, which holds a *cache* built off this /// same client rather than the client. `None` under the same /// "no forge configured on this host" shape every other /// forge-backed field here uses. @@ -1370,8 +1372,9 @@ fn broken_name_rules( } /// Whether `agent` is already in the swarm roster, or why that could not be -/// told. Asked only of a name that breaks a rule, so an ordinary creation -/// still never waits on the bridge. +/// told. `create_agent` asks only of a name that breaks a rule, so an +/// ordinary creation still never waits on the bridge; `make_forge_admin` +/// asks of every name. async fn in_roster(auth: Option<&auth::AuthBridge>, agent: &str) -> Result { let Some(auth) = auth else { return Err("no identity bridge is configured".to_owned()); @@ -1828,6 +1831,113 @@ async fn mint_agent_forge_token( Ok(Json(MintAgentForgeTokenResponse { node_id: id.get() })) } +/// What `POST /api/forge/users/{name}/admin` found. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "snake_case")] +enum ForgeAdminChange { + Promoted, + AlreadyAdmin, +} + +/// Success body of `POST /api/forge/users/{name}/admin`. +#[derive(Serialize, Deserialize, ToSchema)] +struct MakeForgeAdminResponse { + change: ForgeAdminChange, +} + +/// Make an existing human forge account a site admin. Idempotent. +/// +/// Never creates the account: a human's is made by their first SSO login to +/// the forge, and until then this answers 404. Refuses an agent's name: a +/// site admin can create repos whatever its `max_repo_creation`, and an +/// agent's account is locked to 0 so its token cannot create a repo and +/// self-merge in it. +#[utoipa::path( + post, + path = "/api/forge/users/{name}/admin", + params(("name" = String, Path, description = "forge username")), + responses( + (status = 200, description = "the account is a site admin", body = MakeForgeAdminResponse), + (status = 400, description = "`name` is not a valid identifier (problem+json)", body = String), + (status = 404, description = "no such account: the user has not logged in via SSO yet (problem+json)", body = String), + (status = 409, description = "`name` is an agent (problem+json)", body = String), + (status = 500, description = "the forge refused the read or the edit (problem+json)", body = String), + (status = 503, description = "no forge configured, or the agent roster could not be read (problem+json)", body = String), + ), + tag = "forge" +)] +async fn make_forge_admin( + State(state): State, + Path(name): Path, +) -> Result, problem_details::ProblemDetails> { + let name = hive_types::Ident::parse(&name) + .map_err(|reason| error_problem(axum::http::StatusCode::BAD_REQUEST, reason))? + .into_string(); + let Some(forge) = state.forge.clone() else { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + "no forge is configured on this host", + )); + }; + let is_agent = in_roster(state.auth.as_deref(), &name).await; + admin_unless_agent(&name, is_agent, || async { + forge.make_site_admin(&name).await + }) + .await +} + +/// Run `promote` only if `name` is known not to be an agent, and answer with +/// what it found. +/// +/// An unreadable roster refuses too: a site admin is the one thing an agent's +/// account must never become, so "could not tell" is not "no". +async fn admin_unless_agent( + name: &str, + is_agent: Result, + promote: F, +) -> Result, problem_details::ProblemDetails> +where + F: FnOnce() -> Fut, + Fut: std::future::Future>, +{ + match is_agent { + Ok(false) => {} + Ok(true) => { + return Err(error_problem( + axum::http::StatusCode::CONFLICT, + &format!("{name:?} is an agent; an agent's forge account is never a site admin"), + )); + } + Err(e) => { + return Err(error_problem( + axum::http::StatusCode::SERVICE_UNAVAILABLE, + &format!("cannot tell whether {name:?} is an agent, so refusing: {e}"), + )); + } + } + let plan = promote().await.map_err(|e| { + error_problem( + axum::http::StatusCode::INTERNAL_SERVER_ERROR, + &format!("{e:#}"), + ) + })?; + let change = match plan { + forge::site_admin::Plan::NoSuchUser => { + return Err(error_problem( + axum::http::StatusCode::NOT_FOUND, + &format!( + "the forge has no user {name:?}: the user has not logged in via SSO yet. \ + The forge creates the account on their first authelia login; run this \ + again after it" + ), + )); + } + forge::site_admin::Plan::AlreadyAdmin => ForgeAdminChange::AlreadyAdmin, + forge::site_admin::Plan::Promote => ForgeAdminChange::Promoted, + }; + Ok(Json(MakeForgeAdminResponse { change })) +} + /// Every agent with an open config PR, in one response — the bulk /// counterpart to [`get_agent_config_pr`]. swarm-ui's config-PR table needs /// every agent's status to render, and fetching them one at a time doesn't @@ -2175,6 +2285,7 @@ fn build_app(state: AppState) -> axum::Router { .routes(routes!(create_agent)) .routes(routes!(mint_agent_identity)) .routes(routes!(mint_agent_forge_token)) + .routes(routes!(make_forge_admin)) .routes(routes!(get_agents)) .routes(routes!(get_agents_status)) .routes(routes!(set_agent_state)) @@ -2516,6 +2627,94 @@ mod tests { assert!(warnings[0].contains("bridge down"), "{warnings:?}"); } + /// `admin_unless_agent` with a `promote` that records whether it ran and + /// answers `plan`. + async fn forge_admin_route( + is_agent: Result, + plan: crate::forge::site_admin::Plan, + ) -> ( + Result, problem_details::ProblemDetails>, + bool, + ) { + let ran = std::sync::atomic::AtomicBool::new(false); + let result = super::admin_unless_agent("atlas", is_agent, || async { + ran.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(plan) + }) + .await; + (result, ran.into_inner()) + } + + /// An agent's name never reaches the forge. + #[tokio::test] + async fn the_forge_admin_route_refuses_an_agent() { + let (result, ran) = + forge_admin_route(Ok(true), crate::forge::site_admin::Plan::Promote).await; + let Err(problem) = result else { + panic!("an agent's name must be refused"); + }; + assert!(!ran, "the account must not be touched"); + assert_eq!(problem.status, Some(axum::http::StatusCode::CONFLICT)); + } + + /// A roster that can't be read can't vouch for the name, so the route + /// refuses rather than guessing "not an agent". + #[tokio::test] + async fn the_forge_admin_route_refuses_on_an_unreadable_roster() { + let (result, ran) = forge_admin_route( + Err("bridge down".to_owned()), + crate::forge::site_admin::Plan::Promote, + ) + .await; + let Err(problem) = result else { + panic!("an unreadable roster must refuse"); + }; + assert!(!ran, "the account must not be touched"); + assert_eq!( + problem.status, + Some(axum::http::StatusCode::SERVICE_UNAVAILABLE) + ); + } + + /// A missing account is a 404 that tells the operator what to wait for. + #[tokio::test] + async fn the_forge_admin_route_names_the_missing_sso_login() { + let (result, ran) = + forge_admin_route(Ok(false), crate::forge::site_admin::Plan::NoSuchUser).await; + let Err(problem) = result else { + panic!("a missing account must not report success"); + }; + assert!(ran); + assert_eq!(problem.status, Some(axum::http::StatusCode::NOT_FOUND)); + let rendered = format!("{problem:?}"); + assert!( + rendered.contains("has not logged in via SSO yet"), + "{rendered}" + ); + } + + #[tokio::test] + async fn the_forge_admin_route_promotes_a_non_agent() { + let (result, ran) = + forge_admin_route(Ok(false), crate::forge::site_admin::Plan::Promote).await; + let Ok(axum::Json(resp)) = result else { + panic!("a non-agent must be promoted"); + }; + assert!(ran); + assert_eq!(resp.change, super::ForgeAdminChange::Promoted); + } + + /// Idempotent: a second run succeeds and says nothing changed. + #[tokio::test] + async fn the_forge_admin_route_succeeds_for_an_admin() { + let (result, _) = + forge_admin_route(Ok(false), crate::forge::site_admin::Plan::AlreadyAdmin).await; + let Ok(axum::Json(resp)) = result else { + panic!("an existing admin must succeed"); + }; + assert_eq!(resp.change, super::ForgeAdminChange::AlreadyAdmin); + } + /// The backfill route's roster check, asserted by effect for the same /// reason its sibling above is: a refusal that queued first would still /// re-mint the agent's certificate, which every running agent on the