From bbf931207f7e6d335f8a1820390eec3ed4d9d583 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 2 Oct 2026 23:31:39 +0200 Subject: [PATCH] swarm UI: delete linked accounts Each row of an agent's linked accounts, except its own `main` matrix account, gets a delete action. swarm-controller serves DELETE beside each PUT (matrix-accounts/{account}, forge-accounts/{label}, github-account), answers 404 for an account the store does not hold, refuses `main`, and removes every version through `delete_all_versions`. The matrix confirmation has a revoke checkbox, off by default: the controller logs the stored token out at its homeserver first, and keeps the account when that fails or no homeserver is stored. The controller's policy gains `delete` on each agent's `metadata/.../matrix/+`, `forge/+` and `github-token`, pinned in bao-grants.nix. Refs #4855 --- docs/swarm/ui.md | 27 + .../src/pages/agents/LinkedAccounts.tsx | 146 +++++- nix/host-modules/swarm-bao.nix | 14 +- nix/module-eval/bao-grants.nix | 20 +- swarm-controller/src/linked_accounts.rs | 482 +++++++++++++++++- swarm-controller/src/main.rs | 15 +- swarm-controller/src/matrix_account.rs | 36 +- 7 files changed, 705 insertions(+), 35 deletions(-) diff --git a/docs/swarm/ui.md b/docs/swarm/ui.md index 1129e2fe..cf7bc0c9 100644 --- a/docs/swarm/ui.md +++ b/docs/swarm/ui.md @@ -60,6 +60,33 @@ a link dialog closes. none. What the token needs and how the agent uses it: [GitHub accounts](../integrations/github.md). +#### Deleting a linked account + +Every row except the matrix `main` row has a **delete** action. Its +confirmation names the account and its host, and confirming sends one +request: + +| kind | route | +| ------- | ------------------------------------------------------------------- | +| matrix | `DELETE /api/hives/{hive}/agents/{agent}/matrix-accounts/{account}` | +| forgejo | `DELETE /api/hives/{hive}/agents/{agent}/forge-accounts/{label}` | +| github | `DELETE /api/hives/{hive}/agents/{agent}/github-account` | + +swarm-controller removes every version of the entry from the swarm secret +store, and answers 404 when the store holds nothing there. It refuses `main`: the +swarm mints that account and would re-mint it. The panel requests the list +again after a delete. + +The matrix confirmation has a checkbox, off by default, that logs the token +out at its homeserver first (`?revoke=true`). If the homeserver doesn't +confirm the logout, or the account has no homeserver stored, the account stays +in the store and the dialog shows why. Deleting a forge account or GitHub token +leaves the token valid at its provider; revoke it there. + +The agent isn't told about a delete. Its matrix daemon drops the account when +it next lists the store, within two minutes. Its forge and GitHub units never +delete a file, so a token already fetched into `` stays there. + Where each credential lives and who reads it: [`credentials.md`](credentials.md). diff --git a/frontend/packages/swarm-ui/src/pages/agents/LinkedAccounts.tsx b/frontend/packages/swarm-ui/src/pages/agents/LinkedAccounts.tsx index ecf17eba..33f123c1 100644 --- a/frontend/packages/swarm-ui/src/pages/agents/LinkedAccounts.tsx +++ b/frontend/packages/swarm-ui/src/pages/agents/LinkedAccounts.tsx @@ -1,16 +1,19 @@ // — the accounts linked to one agent, one row each: kind, -// name, host. Reads `GET /api/hives/{hive}/agents/{agent}/linked-accounts`, -// which carries names and hosts only, never a credential. +// name, host, and a delete action on every row but the agent's own account. +// Reads `GET /api/hives/{hive}/agents/{agent}/linked-accounts`, which carries +// names and hosts only, never a credential. // -// Fetched on mount and again whenever `version` changes; `AgentsPage` bumps -// it when a link dialog closes, so an account linked there shows up without -// waiting for a reload. +// Fetched on mount, after a delete, and whenever `version` changes; +// `AgentsPage` bumps it when a link dialog closes, so an account linked there +// shows up without waiting for a reload. // // Its state belongs to one agent: callers key it by hive and agent, so // switching agents remounts it instead of showing the previous agent's rows. import { useEffect, useState } from "preact/hooks"; +import { ApiErrorPanel } from "@hive/shared/api-error-panel.js"; import { readApiError, type ProblemDetails } from "@hive/shared/api-error.js"; import { Badge } from "@hive/shared/badge.js"; +import { ConfirmDialog } from "../../ui/confirm-dialog/ConfirmDialog.js"; import "./LinkedAccounts.css"; type AccountKind = "matrix" | "forgejo" | "github"; @@ -23,6 +26,19 @@ interface LinkedAccount { reserved: boolean; } +// The DELETE route for one account, beside the PUT that links it. +function accountUrl(hive: string, agent: string, a: LinkedAccount): string { + const base = `/api/hives/${encodeURIComponent(hive)}/agents/${encodeURIComponent(agent)}`; + switch (a.kind) { + case "matrix": + return `${base}/matrix-accounts/${encodeURIComponent(a.name)}`; + case "forgejo": + return `${base}/forge-accounts/${encodeURIComponent(a.name)}`; + case "github": + return `${base}/github-account`; + } +} + export function LinkedAccounts({ hive, agent, @@ -34,6 +50,14 @@ export function LinkedAccounts({ }) { const [accounts, setAccounts] = useState(null); const [error, setError] = useState(null); + // Bumped after a delete, so the list is fetched again. + const [reload, setReload] = useState(0); + // The row whose delete confirmation is open; null means closed. + const [deleteTarget, setDeleteTarget] = useState(null); + // Matrix only: log the token out at its homeserver before deleting. + const [revoke, setRevoke] = useState(false); + const [deleting, setDeleting] = useState(false); + const [deleteError, setDeleteError] = useState(null); useEffect(() => { let cancelled = false; @@ -55,7 +79,79 @@ export function LinkedAccounts({ return () => { cancelled = true; }; - }, [hive, agent, version]); + }, [hive, agent, version, reload]); + + function openDelete(a: LinkedAccount) { + setDeleteTarget(a); + setRevoke(false); + setDeleteError(null); + } + + async function confirmDelete() { + if (!deleteTarget) return; + setDeleting(true); + try { + const url = accountUrl(hive, agent, deleteTarget); + const r = await fetch( + deleteTarget.kind === "matrix" && revoke ? `${url}?revoke=true` : url, + { method: "DELETE" }, + ); + if (!r.ok) { + setDeleteError(await readApiError(r)); + return; + } + setDeleteTarget(null); + setReload((n) => n + 1); + } catch (e: unknown) { + setDeleteError({ detail: String(e) }); + } finally { + setDeleting(false); + } + } + + const dialog = ( + setDeleteTarget(null)} + onConfirm={() => void confirmDelete()} + confirmLabel="delete" + confirmDisabled={deleting} + > + {deleteTarget ? ( + <> +

+ Delete the {deleteTarget.kind} account{" "} + {deleteTarget.name} + {deleteTarget.host ? ( + <> + {" "} + on {deleteTarget.host} + + ) : null}{" "} + from {agent}? Every stored version of its token is + removed from the swarm secret store. The agent is not told. +

+ {deleteTarget.kind === "matrix" ? ( + + ) : null} + {deleteError ? ( + + ) : null} + + ) : null} + + ); if (error) { return ( @@ -71,19 +167,29 @@ export function LinkedAccounts({ return none linked; } return ( -
    - {accounts.map((a) => ( -
  • - - {a.host ?? "—"} - {a.reserved ? ( - - ) : null} -
    • - ))} -
+ <> +
    + {accounts.map((a) => ( +
  • + + {a.host ?? "—"} + {a.reserved ? ( + + ) : ( + openDelete(a)} + /> + )} +
    • + ))} +
+ {dialog} + ); } diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix index 976d97c2..fba463cd 100644 --- a/nix/host-modules/swarm-bao.nix +++ b/nix/host-modules/swarm-bao.nix @@ -365,7 +365,7 @@ let # back before writing so a re-run keeps the value a live agent already holds # instead of rotating it. `metadata/` is the revocation half: `delete` on # `data/` only soft-deletes the newest version, and `+` being one path segment - # keeps this to the queue leaf alone. + # keeps this to the queue leaf alone, and each linked account to its one leaf. # Each hive's matrix sender token (`matrix_account::hive_sender`) grants `read` # for the same keep-if-live reason, and `+` for the same glob-only-as-last-segment reason. # @@ -408,6 +408,18 @@ let capabilities = ["list"] } + path "${credentialMountPath}/metadata/swarm/agents/+/matrix/+" { + capabilities = ["delete"] + } + + path "${credentialMountPath}/metadata/swarm/agents/+/forge/+" { + capabilities = ["delete"] + } + + path "${credentialMountPath}/metadata/swarm/agents/+/github-token" { + capabilities = ["delete"] + } + path "${credentialMountPath}/data/swarm/hives/+/matrix/sender-token" { capabilities = ["create", "read", "update"] } diff --git a/nix/module-eval/bao-grants.nix b/nix/module-eval/bao-grants.nix index 8ce59cb3..4156e26c 100644 --- a/nix/module-eval/bao-grants.nix +++ b/nix/module-eval/bao-grants.nix @@ -1201,7 +1201,8 @@ let # itself, so each stanza reaches that one directory: not the agent's # other keys, not `agents/` itself, not anything below. Pinned as whole # stanzas, and as the only metadata stanzas under `agents/` beside the - # queue revocation, so a widened path or an added capability fails. + # queue revocation and the account deletes, so a widened path or an added + # capability fails. name = "the controller may list each agent's matrix and forge accounts, and nothing else under agents"; ok = let @@ -1210,7 +1211,22 @@ let in lib.hasInfix "path \"secret/metadata/swarm/agents/+/matrix\" {\n capabilities = [\"list\"]\n}" s && lib.hasInfix "path \"secret/metadata/swarm/agents/+/forge\" {\n capabilities = [\"list\"]\n}" s - && stanzas == 3; + && stanzas == 6; + } + { + # `linked_accounts` deletes one matrix account, forge account or GitHub + # token through `delete_all_versions`, which addresses its metadata path. + # A trailing `+` is one segment, so each stanza reaches the accounts + # directly in that directory and nothing deeper. Pinned as whole stanzas, + # so `list` or `read` here, which would expose version history, fails. + name = "the controller may delete each agent's linked accounts, and nothing else of theirs"; + ok = + let + s = baoGrantHere.systemd.services.swarm-bao-controller-policy.script; + in + lib.hasInfix "path \"secret/metadata/swarm/agents/+/matrix/+\" {\n capabilities = [\"delete\"]\n}" s + && lib.hasInfix "path \"secret/metadata/swarm/agents/+/forge/+\" {\n capabilities = [\"delete\"]\n}" s + && lib.hasInfix "path \"secret/metadata/swarm/agents/+/github-token\" {\n capabilities = [\"delete\"]\n}" s; } { # The swarm appservice token is a homeserver-admin credential. The diff --git a/swarm-controller/src/linked_accounts.rs b/swarm-controller/src/linked_accounts.rs index cb820e67..655e4984 100644 --- a/swarm-controller/src/linked_accounts.rs +++ b/swarm-controller/src/linked_accounts.rs @@ -1,5 +1,5 @@ //! The accounts linked to one agent, as kind, name and host: what the swarm -//! UI's agent panel lists. +//! UI's agent panel lists, and deletes. //! //! Read from the paths [`crate::matrix_account`], [`crate::forge_account`] and //! [`crate::github_account`] write, plus the agent's own `main` matrix account, @@ -8,18 +8,23 @@ //! [`LinkedAccount`] has no field a credential could land in. //! //! Naming the matrix accounts and forge labels takes `list` on each agent's -//! `matrix` and `forge` metadata directories, which `controllerPolicyText` in -//! `nix/host-modules/swarm-bao.nix` grants on those two directories alone. +//! `matrix` and `forge` metadata directories, and deleting an account takes +//! `delete` on its metadata path. `controllerPolicyText` in +//! `nix/host-modules/swarm-bao.nix` grants both on those paths alone. +//! +//! A delete removes the store entry only, and the agent is not told. Its +//! GitHub and forge token files stay in place, since the units that write them +//! never delete one; its matrix daemon drops the account when it next re-lists. use std::future::Future; use axum::Json; use axum::extract::State; use axum::http::StatusCode; -use serde::Serialize; use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; use swarm_secret_client::{Error, SecretStore, forge, github, matrix}; -use utoipa::ToSchema; +use utoipa::{IntoParams, ToSchema}; use super::{AppState, error_problem, swarm_hive}; @@ -52,7 +57,8 @@ pub struct LinkedAccount { reserved: bool, } -/// The store reads a listing makes, so a test can stand in for the store. +/// The store calls a listing or a delete makes, so a test can stand in for the +/// store. pub(crate) trait AccountStore { /// As [`SecretStore::list`]. fn list(&self, dir: &str) -> impl Future, Error>> + Send; @@ -62,6 +68,9 @@ pub(crate) trait AccountStore { &self, path: &str, ) -> impl Future, Error>> + Send; + + /// As [`SecretStore::delete_all_versions`]. + fn delete_all_versions(&self, path: &str) -> impl Future> + Send; } impl AccountStore for SecretStore { @@ -75,6 +84,10 @@ impl AccountStore for SecretStore { ) -> Result, Error> { SecretStore::read_optional(self, path).await } + + async fn delete_all_versions(&self, path: &str) -> Result<(), Error> { + SecretStore::delete_all_versions(self, path).await + } } /// The object names directly under `dir`. A key ending in `/` is a directory @@ -176,23 +189,270 @@ pub async fn get_linked_accounts( Ok(Json(accounts)) } +/// Why an account was not deleted. +#[derive(Debug)] +pub(crate) enum Removal { + /// Nothing is stored at the account's path. + NotFound, + /// Revoking was asked for, and the account has no homeserver to revoke at. + NoHomeserver, + /// The homeserver did not revoke the token. + Revoke(String), + /// The store refused or could not be reached. + Store(Error), +} + +impl Removal { + fn problem(&self) -> problem_details::ProblemDetails { + match self { + Self::NotFound => error_problem(StatusCode::NOT_FOUND, "no such account is stored"), + Self::NoHomeserver => error_problem( + StatusCode::CONFLICT, + "no homeserver is stored for this account, so its token cannot be revoked; \ + the account was not deleted", + ), + Self::Revoke(e) => error_problem( + StatusCode::BAD_GATEWAY, + &format!( + "the homeserver did not revoke the token, so the account was not deleted: {e}" + ), + ), + Self::Store(e) => error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()), + } + } +} + +/// The object at `path`, or [`Removal::NotFound`] when nothing is stored there. +async fn stored( + store: &impl AccountStore, + path: &str, +) -> Result { + store + .read_optional(path) + .await + .map_err(Removal::Store)? + .ok_or(Removal::NotFound) +} + +/// Delete every version of `path`. +async fn remove(store: &impl AccountStore, path: &str) -> Result<(), Removal> { + store + .delete_all_versions(path) + .await + .map_err(Removal::Store) +} + +/// Delete the matrix account at `path`. With `revoke`, its token is first +/// handed to `revoke` with the stored homeserver, and the entry stays when that +/// fails. +async fn remove_matrix( + store: &impl AccountStore, + path: &str, + revoke: Option, +) -> Result<(), Removal> +where + F: FnOnce(String, String) -> Fut, + Fut: Future>, +{ + let credential: matrix::Credential = stored(store, path).await?; + if let Some(revoke) = revoke { + let homeserver = credential.homeserver.ok_or(Removal::NoHomeserver)?; + revoke(homeserver, credential.value) + .await + .map_err(Removal::Revoke)?; + } + remove(store, path).await +} + +/// Delete the forge account or GitHub token at `path`. +async fn remove_plain( + store: &impl AccountStore, + path: &str, +) -> Result<(), Removal> { + stored::(store, path).await?; + remove(store, path).await +} + +/// `delete_matrix_account`'s query. +#[derive(Deserialize, IntoParams)] +pub struct DeleteMatrixAccountQuery { + /// Log the stored token out at its homeserver before deleting. When that + /// fails, the account is not deleted. + #[serde(default)] + revoke: bool, +} + +/// Delete an agent's external matrix account from the store, every version of +/// it. +#[utoipa::path( + delete, + path = "/api/hives/{hive}/agents/{agent}/matrix-accounts/{account}", + params( + ("hive" = String, Path, description = "hive the agent runs on"), + ("agent" = String, Path, description = "agent the account belongs to"), + ("account" = String, Path, description = "the account to delete"), + DeleteMatrixAccountQuery, + ), + responses( + (status = 204, description = "deleted"), + (status = 400, description = "a name is not an identifier, the account name is not a single path segment, the account is 'main' (reserved), or the hive is not in this swarm (problem+json)", body = String), + (status = 404, description = "no such account is stored (problem+json)", body = String), + (status = 409, description = "revoke was asked for and the account has no homeserver stored; not deleted (problem+json)", body = String), + (status = 502, description = "revoke was asked for and the homeserver did not revoke the token; not deleted (problem+json)", body = String), + (status = 500, description = "the store could not be read or written (problem+json)", body = String), + ), + tag = "agents" +)] +pub async fn delete_matrix_account( + State(state): State, + axum::extract::Path((hive, agent, account)): axum::extract::Path<(String, String, String)>, + axum::extract::Query(query): axum::extract::Query, +) -> Result { + let hive = swarm_hive(&state, &hive).map_err(|(s, d)| error_problem(s, &d))?; + let agent = hive_types::Ident::parse(&agent) + .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? + .into_string(); + let path = matrix::account_path(&agent, &account) + .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; + // `agent_token` re-mints `main` on its next reconcile, so deleting it + // would only rotate the agent's own token. + if crate::matrix_account::is_reserved_account(&account) { + return Err(error_problem( + StatusCode::BAD_REQUEST, + "'main' is the agent's own account, which the swarm mints — it cannot be deleted.", + )); + } + + let store = crate::store::connect().await.map_err(|e| { + tracing::warn!(error = %e, "connecting to the swarm secret store failed"); + error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) + })?; + let revoke = query + .revoke + .then_some(|homeserver: String, token: String| async move { + crate::matrix_account::matrix_logout(&homeserver, &token).await + }); + remove_matrix(&store, &path, revoke).await.map_err(|e| { + tracing::warn!(%hive, %agent, %account, error = ?e, "deleting the matrix account failed"); + e.problem() + })?; + + tracing::info!(%hive, %agent, %account, revoked = query.revoke, "matrix account deleted"); + Ok(StatusCode::NO_CONTENT) +} + +/// Delete an agent's external forge account from the store, every version of +/// it. +#[utoipa::path( + delete, + path = "/api/hives/{hive}/agents/{agent}/forge-accounts/{label}", + params( + ("hive" = String, Path, description = "hive the agent runs on"), + ("agent" = String, Path, description = "agent the account belongs to"), + ("label" = String, Path, description = "the account's label"), + ), + responses( + (status = 204, description = "deleted"), + (status = 400, description = "the agent or label is not an identifier, or the hive is not in this swarm (problem+json)", body = String), + (status = 404, description = "no such account is stored (problem+json)", body = String), + (status = 500, description = "the store could not be read or written (problem+json)", body = String), + ), + tag = "agents" +)] +pub async fn delete_forge_account( + State(state): State, + axum::extract::Path((hive, agent, label)): axum::extract::Path<(String, String, String)>, +) -> Result { + let hive = swarm_hive(&state, &hive).map_err(|(s, d)| error_problem(s, &d))?; + let agent = hive_types::Ident::parse(&agent) + .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? + .into_string(); + let label = hive_types::Ident::parse(&label) + .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? + .into_string(); + let path = forge::account_path(&agent, &label) + .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; + + let store = crate::store::connect().await.map_err(|e| { + tracing::warn!(error = %e, "connecting to the swarm secret store failed"); + error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) + })?; + remove_plain::(&store, &path) + .await + .map_err(|e| { + tracing::warn!(%hive, %agent, %label, error = ?e, "deleting the forge account failed"); + e.problem() + })?; + + tracing::info!(%hive, %agent, %label, "forge account deleted"); + Ok(StatusCode::NO_CONTENT) +} + +/// Delete an agent's GitHub token from the store, every version of it. +#[utoipa::path( + delete, + path = "/api/hives/{hive}/agents/{agent}/github-account", + params( + ("hive" = String, Path, description = "hive the agent runs on"), + ("agent" = String, Path, description = "agent the token belongs to"), + ), + responses( + (status = 204, description = "deleted"), + (status = 400, description = "the agent is not an identifier, or the hive is not in this swarm (problem+json)", body = String), + (status = 404, description = "no token is stored (problem+json)", body = String), + (status = 500, description = "the store could not be read or written (problem+json)", body = String), + ), + tag = "agents" +)] +pub async fn delete_github_account( + State(state): State, + axum::extract::Path((hive, agent)): axum::extract::Path<(String, String)>, +) -> Result { + let hive = swarm_hive(&state, &hive).map_err(|(s, d)| error_problem(s, &d))?; + let agent = hive_types::Ident::parse(&agent) + .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? + .into_string(); + let path = github::account_path(&agent) + .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; + + let store = crate::store::connect().await.map_err(|e| { + tracing::warn!(error = %e, "connecting to the swarm secret store failed"); + error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) + })?; + remove_plain::(&store, &path) + .await + .map_err(|e| { + tracing::warn!(%hive, %agent, error = ?e, "deleting the github token failed"); + e.problem() + })?; + + tracing::info!(%hive, %agent, "github token deleted"); + Ok(StatusCode::NO_CONTENT) +} + #[cfg(test)] mod tests { use std::collections::BTreeMap; + use std::sync::Mutex; use serde::de::DeserializeOwned; use serde_json::{Value, json}; use swarm_secret_client::Error; - use super::{AccountKind, AccountStore, LinkedAccount, linked_accounts}; + use super::{ + AccountKind, AccountStore, LinkedAccount, Removal, linked_accounts, remove_matrix, + remove_plain, + }; + use swarm_secret_client::{forge, github}; /// Objects by path. A list answers the next segment of every path under /// the directory, with a trailing `/` when it goes deeper, as the store - /// does. + /// does. A delete is recorded in `deleted` and leaves `objects` as it is. #[derive(Default)] struct FakeStore { objects: BTreeMap, denied: bool, + deleted: Mutex>, } impl FakeStore { @@ -233,6 +493,17 @@ mod tests { .get(path) .map(|v| serde_json::from_value(v.clone()).expect("fixture decodes"))) } + + async fn delete_all_versions(&self, path: &str) -> Result<(), Error> { + if self.denied { + return Err(Error::MissingEnv("BAO_ADDR")); + } + self.deleted + .lock() + .expect("not poisoned") + .push(path.to_owned()); + Ok(()) + } } fn row(kind: AccountKind, name: &str, host: Option<&str>, reserved: bool) -> LinkedAccount { @@ -350,4 +621,199 @@ mod tests { }; assert!(linked_accounts(&store, "atlas").await.is_err()); } + + /// The `revoke` a delete without the checkbox passes. + type NoRevoke = fn(String, String) -> std::future::Ready>; + + fn deleted(store: &FakeStore) -> Vec { + store.deleted.lock().expect("not poisoned").clone() + } + + #[tokio::test] + async fn each_kind_deletes_the_stored_account() { + let store = every_kind(); + remove_matrix( + &store, + "swarm/agents/atlas/matrix/catgirl", + None::, + ) + .await + .expect("stored"); + remove_plain::(&store, "swarm/agents/atlas/forge/codeberg") + .await + .expect("stored"); + remove_plain::(&store, "swarm/agents/atlas/github-token") + .await + .expect("stored"); + assert_eq!( + deleted(&store), + [ + "swarm/agents/atlas/matrix/catgirl", + "swarm/agents/atlas/forge/codeberg", + "swarm/agents/atlas/github-token", + ] + ); + } + + #[tokio::test] + async fn an_unknown_account_is_a_404_and_nothing_is_deleted() { + let store = FakeStore::default(); + let results = [ + remove_matrix( + &store, + "swarm/agents/atlas/matrix/catgirl", + None::, + ) + .await, + remove_plain::(&store, "swarm/agents/atlas/forge/codeberg").await, + remove_plain::(&store, "swarm/agents/atlas/github-token").await, + ]; + for result in results { + let removal = result.expect_err("nothing is stored"); + assert!(matches!(removal, Removal::NotFound), "{removal:?}"); + assert_eq!( + removal.problem().status, + Some(axum::http::StatusCode::NOT_FOUND) + ); + } + assert!(deleted(&store).is_empty()); + } + + #[tokio::test] + async fn revoking_logs_out_the_stored_token_at_its_homeserver_then_deletes() { + let store = every_kind(); + let asked = Mutex::new(None); + remove_matrix( + &store, + "swarm/agents/atlas/matrix/catgirl", + Some(|homeserver: String, token: String| { + *asked.lock().expect("not poisoned") = Some((homeserver, token)); + std::future::ready(Ok(())) + }), + ) + .await + .expect("revoked"); + assert_eq!( + asked.into_inner().expect("not poisoned"), + Some(( + "https://matrix.example.org".to_owned(), + "t0k3n-cat".to_owned() + )) + ); + assert_eq!(deleted(&store), ["swarm/agents/atlas/matrix/catgirl"]); + } + + #[tokio::test] + async fn a_failed_revoke_keeps_the_account_and_says_so() { + let store = every_kind(); + let removal = remove_matrix( + &store, + "swarm/agents/atlas/matrix/catgirl", + Some(|_: String, _: String| { + std::future::ready(Err("/logout HTTP 502 Bad Gateway: down".to_owned())) + }), + ) + .await + .expect_err("the homeserver refused"); + assert!(deleted(&store).is_empty()); + let problem = removal.problem(); + assert_eq!(problem.status, Some(axum::http::StatusCode::BAD_GATEWAY)); + let detail = problem.detail.expect("a detail"); + assert!(detail.contains("not deleted"), "{detail}"); + assert!(detail.contains("down"), "{detail}"); + } + + #[tokio::test] + async fn revoking_an_account_with_no_homeserver_keeps_it() { + let store = every_kind(); + let removal = remove_matrix( + &store, + "swarm/agents/atlas/matrix/old", + Some( + |_: String, _: String| -> std::future::Ready> { + panic!("there is no homeserver to call") + }, + ), + ) + .await + .expect_err("no homeserver"); + assert!(matches!(removal, Removal::NoHomeserver), "{removal:?}"); + assert!(deleted(&store).is_empty()); + } + + #[tokio::test] + async fn a_store_that_refuses_a_delete_is_an_error() { + let store = FakeStore { + denied: true, + ..every_kind() + }; + let removal = remove_plain::(&store, "swarm/agents/atlas/github-token") + .await + .expect_err("denied"); + assert!(matches!(removal, Removal::Store(_)), "{removal:?}"); + } + + /// Bare-minimum `AppState`, as `matrix_account`'s tests build it. + fn state() -> super::super::AppState { + super::super::AppState { + hives: std::sync::Arc::new(vec![super::super::HiveEntry { + name: "pr1ma".to_owned(), + domain: "pr1ma.example".to_owned(), + }]), + links: std::sync::Arc::new(Vec::new()), + status: None, + wanted: None, + agent_status: None, + agent_icons: None, + jobq: std::sync::Arc::new(std::sync::Mutex::new(hive_jobq::scheduler::Scheduler::new( + hive_jobq::Graph::new(), + hive_jobq::resources::ResourceTable::new(), + ))), + webhook_secret: None, + config_prs: None, + swarm_name: None, + auth: None, + forge: None, + create_gate: std::sync::Arc::default(), + } + } + + async fn delete_matrix(account: &str) -> problem_details::ProblemDetails { + super::delete_matrix_account( + axum::extract::State(state()), + axum::extract::Path(("pr1ma".to_owned(), "atlas".to_owned(), account.to_owned())), + axum::extract::Query(super::DeleteMatrixAccountQuery { revoke: true }), + ) + .await + .expect_err("no store is configured in a test") + } + + /// Refused before the store, and before any revoke: with `BAO_*` unset a + /// store connect would answer 500. + #[tokio::test] + async fn the_agents_own_matrix_account_cannot_be_deleted() { + for var in ["BAO_ADDR", "BAO_CLIENT_CERT", "BAO_CLIENT_KEY"] { + assert!( + std::env::var(var).is_err(), + "{var} must be unset for this test to prove anything" + ); + } + let problem = delete_matrix("main").await; + assert_eq!( + problem.status, + Some(axum::http::StatusCode::BAD_REQUEST), + "{problem:?}" + ); + } + + /// The control: any other account name reaches the store connect. + #[tokio::test] + async fn another_matrix_account_reaches_the_store() { + let problem = delete_matrix("catgirl").await; + assert_eq!( + problem.status, + Some(axum::http::StatusCode::INTERNAL_SERVER_ERROR), + "{problem:?}" + ); + } } diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 7612ef38..066c3c86 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -2977,9 +2977,18 @@ fn build_app(state: AppState) -> axum::Router { .routes(routes!(get_agents)) .routes(routes!(get_agents_status)) .routes(routes!(set_agent_state)) - .routes(routes!(matrix_account::put_matrix_account)) - .routes(routes!(forge_account::put_forge_account)) - .routes(routes!(github_account::put_github_account)) + .routes(routes!( + matrix_account::put_matrix_account, + linked_accounts::delete_matrix_account + )) + .routes(routes!( + forge_account::put_forge_account, + linked_accounts::delete_forge_account + )) + .routes(routes!( + github_account::put_github_account, + linked_accounts::delete_github_account + )) .routes(routes!(linked_accounts::get_linked_accounts)) .routes(routes!(get_hive_wanted)) .routes(routes!(term_stream::stream_agent_term)) diff --git a/swarm-controller/src/matrix_account.rs b/swarm-controller/src/matrix_account.rs index 994f8195..05411f98 100644 --- a/swarm-controller/src/matrix_account.rs +++ b/swarm-controller/src/matrix_account.rs @@ -196,7 +196,7 @@ pub async fn put_matrix_account( /// Whether `account` is the agent's own account, which [`agent_token`] mints /// and `nix/agent-modules/matrix.nix` declares per agent — see /// [`put_matrix_account`]'s comment on it for why that route must never write -/// one. +/// one. `linked_accounts::delete_matrix_account` refuses to delete it too. pub(crate) fn is_reserved_account(account: &str) -> bool { account == agent_token::ACCOUNT } @@ -356,6 +356,40 @@ async fn matrix_password_login( Ok((token.to_owned(), uid.to_owned())) } +/// POST `/_matrix/client/v3/logout` with `token`, which +/// invalidates that token at the homeserver. +/// +/// The error names the status and the homeserver's `error` text, never the +/// token. +pub(crate) async fn matrix_logout(homeserver: &str, token: &str) -> Result<(), String> { + let url = format!( + "{}/_matrix/client/v3/logout", + homeserver.trim_end_matches('/') + ); + let resp = http_client()? + .post(&url) + .bearer_auth(token) + .json(&serde_json::json!({})) + .send() + .await + .map_err(|e| http_error("POST /logout", &e))?; + let status = resp.status(); + if status.is_success() { + return Ok(()); + } + let err = resp + .json::() + .await + .ok() + .and_then(|j| { + j.get("error") + .and_then(serde_json::Value::as_str) + .map(str::to_owned) + }) + .unwrap_or_else(|| "logout failed".to_owned()); + Err(format!("/logout HTTP {status}: {err}")) +} + #[cfg(test)] mod tests { use super::{