fix(#3344): type the roster entry's name as Ident, per review
A bare String in a crate whose whole suite types validated names as hive_types::Ident, so a consumer had to re-derive at the boundary what every sibling field gets checked for free. Ident is strictly narrower than the store's charset — [a-z0-9-] against the [A-Za-z0-9._-] the users database allows, because that file holds humans too. Every agent name is a legal Ident by construction (creation parses one before the job is queued), so the narrowing costs real agents nothing. What it does mean is that a human hand-added to the agent group cannot be described as an agent: the reader omits that row and logs it, rather than failing the whole roster or quietly shrinking the answer. The REQUEST keeps its String. That side carries what a caller asked for, and validating it server-side is what lets a bad name be refused with a message instead of failing to deserialise.
This commit is contained in:
parent
e71c9cc431
commit
d58be6834b
6 changed files with 51 additions and 7 deletions
2
Cargo.lock
generated
2
Cargo.lock
generated
|
|
@ -4544,6 +4544,7 @@ version = "0.1.0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
"anyhow",
|
"anyhow",
|
||||||
"axum",
|
"axum",
|
||||||
|
"hive-types",
|
||||||
"reqwest",
|
"reqwest",
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
|
|
@ -4558,6 +4559,7 @@ dependencies = [
|
||||||
name = "swarm-authelia-bridge-sock"
|
name = "swarm-authelia-bridge-sock"
|
||||||
version = "0.1.0"
|
version = "0.1.0"
|
||||||
dependencies = [
|
dependencies = [
|
||||||
|
"hive-types",
|
||||||
"serde",
|
"serde",
|
||||||
"serde_json",
|
"serde_json",
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,7 @@ edition.workspace = true
|
||||||
readme = "README.md"
|
readme = "README.md"
|
||||||
|
|
||||||
[dependencies]
|
[dependencies]
|
||||||
|
hive-types.workspace = true
|
||||||
serde.workspace = true
|
serde.workspace = true
|
||||||
|
|
||||||
[dev-dependencies]
|
[dev-dependencies]
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@
|
||||||
//! file blob — that would invite a last-writer-wins race between independent
|
//! file blob — that would invite a last-writer-wins race between independent
|
||||||
//! callers and duplicate the rendering logic on both sides of the wire.
|
//! callers and duplicate the rendering logic on both sides of the wire.
|
||||||
|
|
||||||
|
use hive_types::Ident;
|
||||||
use serde::{Deserialize, Serialize};
|
use serde::{Deserialize, Serialize};
|
||||||
|
|
||||||
/// A request to the bridge — see the module doc for why this isn't a
|
/// A request to the bridge — see the module doc for why this isn't a
|
||||||
|
|
@ -69,7 +70,20 @@ pub enum BridgeRequest {
|
||||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
pub struct AgentIdentity {
|
pub struct AgentIdentity {
|
||||||
/// The authelia username, which is the agent's name.
|
/// The authelia username, which is the agent's name.
|
||||||
pub name: String,
|
///
|
||||||
|
/// [`Ident`] rather than a bare `String`, so a consumer gets the same
|
||||||
|
/// serde-checked parsing at the socket boundary that every other
|
||||||
|
/// agent-name field in the crate suite does.
|
||||||
|
///
|
||||||
|
/// ⚠️ **It is deliberately NARROWER than what the store accepts.**
|
||||||
|
/// `Ident` is `[a-z0-9-]`; the users database also admits `.`, `_` and
|
||||||
|
/// uppercase, because it holds humans too. Every *agent* name is a valid
|
||||||
|
/// `Ident` by construction — agent creation parses one before the job is
|
||||||
|
/// queued — so the narrowing costs nothing for real agents and refuses to
|
||||||
|
/// describe a human who was hand-added to the agent group as though they
|
||||||
|
/// were one. The reader skips such an entry and logs it; see the bridge's
|
||||||
|
/// `list`.
|
||||||
|
pub name: Ident,
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The bridge's answer to a [`BridgeRequest`].
|
/// The bridge's answer to a [`BridgeRequest`].
|
||||||
|
|
@ -109,6 +123,7 @@ pub enum BridgeResponse {
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{AgentIdentity, BridgeRequest, BridgeResponse};
|
use super::{AgentIdentity, BridgeRequest, BridgeResponse};
|
||||||
|
use hive_types::Ident;
|
||||||
|
|
||||||
/// Pins the external-tag wire shape — a reader off the wire (or a log
|
/// Pins the external-tag wire shape — a reader off the wire (or a log
|
||||||
/// line) should be able to tell the outcomes apart without
|
/// line) should be able to tell the outcomes apart without
|
||||||
|
|
@ -128,7 +143,7 @@ mod tests {
|
||||||
|
|
||||||
let agents = serde_json::to_value(BridgeResponse::Agents {
|
let agents = serde_json::to_value(BridgeResponse::Agents {
|
||||||
agents: vec![AgentIdentity {
|
agents: vec![AgentIdentity {
|
||||||
name: "atlas".to_owned(),
|
name: Ident::parse("atlas").expect("valid ident"),
|
||||||
}],
|
}],
|
||||||
})
|
})
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
@ -149,6 +164,11 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn request_round_trips() {
|
fn request_round_trips() {
|
||||||
|
// Still a `String`: the REQUEST carries whatever the caller asked for
|
||||||
|
// and the bridge validates it server-side, which is what lets an
|
||||||
|
// illegal name be refused with a message rather than failing to
|
||||||
|
// deserialise. Only the ROSTER ENTRY is an `Ident`, because that one
|
||||||
|
// describes something the store already accepted.
|
||||||
let req = BridgeRequest::EnsureAgentIdentity {
|
let req = BridgeRequest::EnsureAgentIdentity {
|
||||||
name: "atlas".to_owned(),
|
name: "atlas".to_owned(),
|
||||||
};
|
};
|
||||||
|
|
@ -180,7 +200,7 @@ mod tests {
|
||||||
#[test]
|
#[test]
|
||||||
fn a_roster_entry_carries_only_the_name() {
|
fn a_roster_entry_carries_only_the_name() {
|
||||||
let entry = serde_json::to_value(AgentIdentity {
|
let entry = serde_json::to_value(AgentIdentity {
|
||||||
name: "atlas".to_owned(),
|
name: Ident::parse("atlas").expect("valid ident"),
|
||||||
})
|
})
|
||||||
.unwrap();
|
.unwrap();
|
||||||
let keys: Vec<&String> = entry.as_object().expect("an object").keys().collect();
|
let keys: Vec<&String> = entry.as_object().expect("an object").keys().collect();
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ name = "swarm-authelia-bridge"
|
||||||
path = "src/main.rs"
|
path = "src/main.rs"
|
||||||
|
|
||||||
[dependencies]
|
[dependencies]
|
||||||
|
hive-types.workspace = true
|
||||||
anyhow.workspace = true
|
anyhow.workspace = true
|
||||||
axum.workspace = true
|
axum.workspace = true
|
||||||
reqwest.workspace = true
|
reqwest.workspace = true
|
||||||
|
|
|
||||||
|
|
@ -312,7 +312,26 @@ fn list(state: &AppState) -> Result<BridgeResponse> {
|
||||||
Ok(BridgeResponse::Agents {
|
Ok(BridgeResponse::Agents {
|
||||||
agents: store::agent_names(&user_store)
|
agents: store::agent_names(&user_store)
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|name| AgentIdentity { name })
|
.filter_map(|name| match hive_types::Ident::parse(&name) {
|
||||||
|
Ok(ident) => Some(AgentIdentity { name: ident }),
|
||||||
|
// The store's charset is wider than an agent name's, because
|
||||||
|
// it holds humans too. Every agent this bridge creates is a
|
||||||
|
// legal `Ident` by construction, so reaching here means a
|
||||||
|
// subject was hand-added to the agent group that cannot be an
|
||||||
|
// agent. Skipped rather than failing the whole roster — one
|
||||||
|
// bad row must not make every other agent invisible — but
|
||||||
|
// logged, because silently shrinking an answer is how a
|
||||||
|
// roster starts lying about who is missing.
|
||||||
|
Err(reason) => {
|
||||||
|
tracing::warn!(
|
||||||
|
subject = %name,
|
||||||
|
%reason,
|
||||||
|
"subject carries the agent group but is not a legal agent name; \
|
||||||
|
omitting it from the roster"
|
||||||
|
);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
.collect(),
|
.collect(),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -77,9 +77,10 @@ impl AuthBridge {
|
||||||
/// facts, and only one of them is a valid render.
|
/// facts, and only one of them is a valid render.
|
||||||
pub async fn list_agent_identities(&self) -> Result<Vec<String>> {
|
pub async fn list_agent_identities(&self) -> Result<Vec<String>> {
|
||||||
match self.request(&BridgeRequest::ListAgentIdentities).await? {
|
match self.request(&BridgeRequest::ListAgentIdentities).await? {
|
||||||
BridgeResponse::Agents { agents } => {
|
BridgeResponse::Agents { agents } => Ok(agents
|
||||||
Ok(agents.into_iter().map(|entry| entry.name).collect())
|
.into_iter()
|
||||||
}
|
.map(|entry| entry.name.into_string())
|
||||||
|
.collect()),
|
||||||
other => {
|
other => {
|
||||||
anyhow::bail!("swarm-authelia-bridge answered a roster request with {other:?}")
|
anyhow::bail!("swarm-authelia-bridge answered a roster request with {other:?}")
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue