swarm: drop the -agent client suffix refusal from authelia and the queue responder
authelia.agentClientSuffix is removed (tombstoned) and the responder takes no --agent-client-suffix: hive-<name>-agent is parsed as hive <name>-agent like any other id. The reserved word agent still keeps a hive from being named *-agent.
This commit is contained in:
parent
6bcffb0ebb
commit
e01770c056
12 changed files with 33 additions and 233 deletions
|
|
@ -86,7 +86,6 @@ mod tests {
|
|||
fn policy() -> Policy {
|
||||
Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
vec!["swarm-controller".to_owned()],
|
||||
vec![],
|
||||
|
|
@ -154,14 +153,8 @@ mod tests {
|
|||
|
||||
#[tokio::test]
|
||||
async fn a_verified_agent_with_no_subjects_configured_is_denied() {
|
||||
let bare = Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
vec![],
|
||||
vec![],
|
||||
)
|
||||
.expect("valid");
|
||||
let bare = Policy::new("hive-".to_owned(), "hive-status".to_owned(), vec![], vec![])
|
||||
.expect("valid");
|
||||
let Presented::Agent(token) = classify("swarm-agent.atlas.Ab9_-zSECRET") else {
|
||||
panic!("an agent token");
|
||||
};
|
||||
|
|
|
|||
|
|
@ -125,21 +125,6 @@ struct Args {
|
|||
#[arg(long, default_value = "hive-")]
|
||||
hive_client_prefix: String,
|
||||
|
||||
/// Suffix appended to a hive's own client id: `hive-<name>-agent`. No
|
||||
/// client is minted with it — agents authenticate with their own secret —
|
||||
/// so the responder refuses it; the suffix is what tells such an id apart
|
||||
/// from the hive `<name>-agent`.
|
||||
///
|
||||
/// A suffix on the hive's id rather than a prefix of its own, because
|
||||
/// `agent-<name>` reads as *the agent called `<name>`* — the one thing
|
||||
/// this identity does not carry.
|
||||
///
|
||||
/// A suffix starts with `-`, so it has to be accepted as a value rather
|
||||
/// than read as a short-flag cluster — without this the default below
|
||||
/// cannot be passed back in on the command line that produced it.
|
||||
#[arg(long, default_value = "-agent", allow_hyphen_values = true)]
|
||||
agent_client_suffix: String,
|
||||
|
||||
/// Client ids allowed to read every hive's status. Repeatable. The
|
||||
/// default is the swarm controller, which is the only reader that exists.
|
||||
#[arg(long = "reader-client", default_values_t = [String::from("swarm-controller")])]
|
||||
|
|
@ -239,7 +224,6 @@ async fn serve(args: Args) -> anyhow::Result<()> {
|
|||
// silently over-broad grant is not.
|
||||
let policy = policy::Policy::new(
|
||||
args.hive_client_prefix.clone(),
|
||||
args.agent_client_suffix.clone(),
|
||||
swarm_queue_client::status::BUCKET.to_owned(),
|
||||
args.reader_clients.clone(),
|
||||
args.hive_publish_subjects.clone(),
|
||||
|
|
@ -607,11 +591,10 @@ mod tests {
|
|||
})
|
||||
}
|
||||
|
||||
/// The argument vector `swarm-nats.nix` actually builds, in the form it
|
||||
/// builds it: every flag space-separated, and the suffix's value opening
|
||||
/// with `-`. Read as a short-flag cluster it aborts before `main` runs,
|
||||
/// and a responder that never starts denies every client — which reaches
|
||||
/// the operator as `authorization violation` on an unrelated process.
|
||||
/// The argument vector `swarm-nats.nix` actually builds, every flag
|
||||
/// space-separated. A parse failure aborts before `main` runs, and a
|
||||
/// responder that never starts denies every client — which reaches the
|
||||
/// operator as `authorization violation` on an unrelated process.
|
||||
#[test]
|
||||
fn the_units_own_argument_vector_parses() {
|
||||
let args = responder([
|
||||
|
|
@ -620,8 +603,6 @@ mod tests {
|
|||
"https://auth.example/api/oidc/introspection",
|
||||
"--hive-client-prefix",
|
||||
"hive-",
|
||||
"--agent-client-suffix",
|
||||
"-agent",
|
||||
"--agent-token-publish-subject",
|
||||
"$SWARM.term.{agent}",
|
||||
"--agent-token-publish-subject",
|
||||
|
|
@ -638,7 +619,6 @@ mod tests {
|
|||
"swarm-nats-auth",
|
||||
])
|
||||
.expect("the unit's own argument vector must parse");
|
||||
assert_eq!(args.agent_client_suffix, "-agent");
|
||||
assert_eq!(args.agent_token_publish_subjects.len(), 6);
|
||||
}
|
||||
|
||||
|
|
@ -704,16 +684,11 @@ mod tests {
|
|||
"https://auth.example/api/oidc/introspection",
|
||||
])
|
||||
.expect("the required flags alone must parse");
|
||||
let policy = policy::Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
vec![],
|
||||
vec![],
|
||||
)
|
||||
.expect("valid")
|
||||
.with_agent_token_subjects(vec!["$SWARM.term.{agent}".to_owned()])
|
||||
.expect("valid");
|
||||
let policy =
|
||||
policy::Policy::new("hive-".to_owned(), "hive-status".to_owned(), vec![], vec![])
|
||||
.expect("valid")
|
||||
.with_agent_token_subjects(vec!["$SWARM.term.{agent}".to_owned()])
|
||||
.expect("valid");
|
||||
let client_secret =
|
||||
oidc_secret::OidcSecret::new(swarm_secret_client::stored_secret::StoredSecret::new(
|
||||
"swarm-nats",
|
||||
|
|
@ -847,35 +822,6 @@ mod tests {
|
|||
assert!(!expired(None, at(1_760_000_001_000)));
|
||||
}
|
||||
|
||||
/// The control for the case above: an ordinary value parses through the
|
||||
/// same flag, so a failure there is about the leading `-` and not about
|
||||
/// the argument being unknown.
|
||||
#[test]
|
||||
fn an_agent_suffix_without_a_leading_dash_parses_too() {
|
||||
let args = responder([
|
||||
"swarm-nats-auth",
|
||||
"--introspection-url",
|
||||
"https://auth.example/api/oidc/introspection",
|
||||
"--agent-client-suffix",
|
||||
"agent",
|
||||
])
|
||||
.expect("an ordinary suffix must parse");
|
||||
assert_eq!(args.agent_client_suffix, "agent");
|
||||
}
|
||||
|
||||
/// The default is the value the deployment passes, so the two cannot
|
||||
/// drift into disagreeing about what an agent client is called.
|
||||
#[test]
|
||||
fn the_default_suffix_is_what_the_module_passes() {
|
||||
let args = responder([
|
||||
"swarm-nats-auth",
|
||||
"--introspection-url",
|
||||
"https://auth.example/api/oidc/introspection",
|
||||
])
|
||||
.expect("the required flags alone must parse");
|
||||
assert_eq!(args.agent_client_suffix, "-agent");
|
||||
}
|
||||
|
||||
/// The verb as `swarm-bao-nats-callout-xkey` runs it, with none of the
|
||||
/// responder's required flags.
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -44,7 +44,6 @@ pub struct Permissions {
|
|||
#[derive(Debug, Clone)]
|
||||
pub struct Policy {
|
||||
hive_prefix: String,
|
||||
agent_suffix: String,
|
||||
bucket: String,
|
||||
readers: Vec<String>,
|
||||
extra_hive_subjects: Vec<String>,
|
||||
|
|
@ -62,16 +61,10 @@ const HIVE_PLACEHOLDER: &str = "{hive}";
|
|||
const AGENT_PLACEHOLDER: &str = "{agent}";
|
||||
|
||||
impl Policy {
|
||||
/// `hive_prefix` is the client-id prefix that marks a hive and
|
||||
/// `agent_suffix` the suffix the responder refuses **on top of** it —
|
||||
/// `hive-<name>` and `hive-<name>-agent`. `bucket` is the KV bucket hives
|
||||
/// report status in, `readers` the client ids allowed to read every hive's
|
||||
/// key, and `extra_hive_subjects` additional subjects a hive may publish to
|
||||
/// (with `{hive}` standing for the hive's own name).
|
||||
///
|
||||
/// The agent id is deliberately the hive's id extended, not a second
|
||||
/// top-level prefix: `agent-<name>` reads as *the agent called `<name>`*,
|
||||
/// which is the one thing this identity does not carry.
|
||||
/// `hive_prefix` is the client-id prefix that marks a hive. `bucket` is the KV
|
||||
/// bucket hives report status in, `readers` the client ids allowed to read
|
||||
/// every hive's key, and `extra_hive_subjects` additional subjects a hive may
|
||||
/// publish to (with `{hive}` standing for the hive's own name).
|
||||
///
|
||||
/// # Errors
|
||||
///
|
||||
|
|
@ -81,14 +74,8 @@ impl Policy {
|
|||
/// whose only purpose is to provide one. Returning a `Result` rather than
|
||||
/// checking at the call site is deliberate: it makes an unscoped policy
|
||||
/// unconstructible instead of merely unlikely.
|
||||
///
|
||||
/// An empty `agent_suffix` is refused for the same reason: every hive id
|
||||
/// would also parse as its own agent id, and the arms are tried in order,
|
||||
/// so the overlap does not error at match time — it silently hands one
|
||||
/// principal the other's grant.
|
||||
pub fn new(
|
||||
hive_prefix: String,
|
||||
agent_suffix: String,
|
||||
bucket: String,
|
||||
readers: Vec<String>,
|
||||
extra_hive_subjects: Vec<String>,
|
||||
|
|
@ -103,21 +90,8 @@ impl Policy {
|
|||
a per-hive namespace"
|
||||
);
|
||||
}
|
||||
// Checked here rather than trusted from nix, because these are two
|
||||
// flags and nothing downstream compares them. An agent id is a hive id
|
||||
// plus this suffix, so an empty one makes `strip_suffix` succeed on
|
||||
// every hive id and the two principals become the same string. The
|
||||
// agent arm runs first and refuses, so every hive would be refused as
|
||||
// its own agents' client.
|
||||
if agent_suffix.is_empty() {
|
||||
anyhow::bail!(
|
||||
"--agent-client-suffix is empty: an agent id is a hive id plus this suffix, \
|
||||
so every hive would also parse as its own agents' client and be refused"
|
||||
);
|
||||
}
|
||||
Ok(Self {
|
||||
hive_prefix,
|
||||
agent_suffix,
|
||||
bucket,
|
||||
readers,
|
||||
extra_hive_subjects,
|
||||
|
|
@ -221,22 +195,6 @@ impl Policy {
|
|||
/// a connected client with no permissions still holds a slot and still
|
||||
/// looks admitted in the logs, which is a worse answer than a refusal.
|
||||
pub fn permissions(&self, client_id: &str) -> Option<Permissions> {
|
||||
// ⚠️ THE AGENT ARM RUNS FIRST, AND THE ORDER IS LOAD-BEARING. An agent
|
||||
// id is a hive id with a suffix, so `hive-foo-agent` satisfies the hive
|
||||
// arm too — it strips to the hive `foo-agent`. Hive-first would
|
||||
// therefore hand every agent its hive's grant, including writing that
|
||||
// hive's status key, and nothing would report it: the client
|
||||
// authenticates, connects, and is simply able to do more than it
|
||||
// should. Most specific wins, so the more specific test is the one
|
||||
// that has to be asked first.
|
||||
//
|
||||
// A hive's shared agent client is refused outright. Agents connect with
|
||||
// their own credential (`Self::agent_token_permissions`); this identity
|
||||
// names the hive and never the agent (see `Self::agent_hive`), so any
|
||||
// grant to it would let one agent publish as its siblings.
|
||||
if self.agent_hive(client_id).is_some() {
|
||||
return None;
|
||||
}
|
||||
if let Some(hive) = self.hive_name(client_id) {
|
||||
return Some(Permissions {
|
||||
publish: self.hive_subjects(hive),
|
||||
|
|
@ -438,26 +396,6 @@ impl Policy {
|
|||
.filter(|name| !name.is_empty())
|
||||
}
|
||||
|
||||
/// The hive whose agents a client id names, when it names one.
|
||||
///
|
||||
/// [`Self::hive_name`]'s caveats all apply — this is that parse with the
|
||||
/// suffix stripped as well, so `hive-foo-agent` answers `foo`. Two more on
|
||||
/// top:
|
||||
///
|
||||
/// - The id carries the hive and never the agent, so this answers *whose
|
||||
/// agents*, never *which agent*.
|
||||
/// - A hive genuinely named `foo-agent` has the client id `hive-foo-agent`,
|
||||
/// which this reads as hive `foo`'s agent client, an id the responder
|
||||
/// refuses. Unguardable from inside this responder, which has no roster
|
||||
/// (see [`Self::hive_name`]) — `swarm-authelia.nix` asserts it at eval,
|
||||
/// where the roster is known, so the deployment fails to build.
|
||||
fn agent_hive<'a>(&self, client_id: &'a str) -> Option<&'a str> {
|
||||
client_id
|
||||
.strip_prefix(&self.hive_prefix)?
|
||||
.strip_suffix(&self.agent_suffix)
|
||||
.filter(|name| !name.is_empty())
|
||||
}
|
||||
|
||||
fn stream(&self) -> String {
|
||||
format!("KV_{}", self.bucket)
|
||||
}
|
||||
|
|
@ -826,7 +764,6 @@ mod tests {
|
|||
fn policy() -> Policy {
|
||||
Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
vec!["swarm-controller".to_owned()],
|
||||
Vec::new(),
|
||||
|
|
@ -1087,7 +1024,6 @@ mod tests {
|
|||
// way to remove it - silently, and only in the deployment that set it.
|
||||
let err = Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
Vec::new(),
|
||||
vec!["$SWARM.events.all".to_owned()],
|
||||
|
|
@ -1106,7 +1042,6 @@ mod tests {
|
|||
// from the option being broken.
|
||||
Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
Vec::new(),
|
||||
vec!["$SWARM.events.{hive}.>".to_owned()],
|
||||
|
|
@ -2201,7 +2136,6 @@ mod tests {
|
|||
// another's events even though its status key is scoped.
|
||||
let p = Policy::new(
|
||||
"hive-".to_owned(),
|
||||
"-agent".to_owned(),
|
||||
"hive-status".to_owned(),
|
||||
Vec::new(),
|
||||
vec!["$SWARM.events.{hive}.>".to_owned()],
|
||||
|
|
@ -2212,38 +2146,6 @@ mod tests {
|
|||
assert!(!g.publish.iter().any(|s| s.contains("{hive}")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn the_hives_shared_agent_client_is_refused() {
|
||||
// Agents connect with their own credential. The shared client names
|
||||
// the hive and never the agent, so any grant to it would let one agent
|
||||
// publish as its siblings.
|
||||
//
|
||||
// 🩸 The agent id satisfies the HIVE parse too (`hive-alpha-agent`
|
||||
// strips to a hive named `alpha-agent`), so a hive-first arm order
|
||||
// would hand this client its hive's grant, status key included, and
|
||||
// nothing would report it. `is_none` here is what holds the order.
|
||||
assert!(policy().permissions("hive-alpha-agent").is_none());
|
||||
// Control: the hive itself is admitted, so the refusal above is the
|
||||
// agent arm answering rather than the prefix failing to match.
|
||||
assert!(policy().permissions("hive-alpha").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_empty_agent_suffix_is_refused() {
|
||||
// 🩸 An agent id is a hive id plus the suffix, so an empty suffix makes
|
||||
// `strip_suffix` succeed on every hive id: the two principals become
|
||||
// one string, and the agent arm, which runs first and refuses, would
|
||||
// refuse every hive. That fails at construction rather than at connect.
|
||||
Policy::new(
|
||||
"hive-".to_owned(),
|
||||
String::new(),
|
||||
"hive-status".to_owned(),
|
||||
Vec::new(),
|
||||
Vec::new(),
|
||||
)
|
||||
.expect_err("an empty agent suffix must not construct");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_agent_token_grant_is_the_agents_own_subjects_and_nothing_else() {
|
||||
let p = policy()
|
||||
|
|
|
|||
Loading…
Reference in a new issue