Compare commits

...
Author SHA1 Message Date
atlas
c290a3b209 fix(#3363): build the test's client without a system trust store
The new regression test constructed `reqwest::Client::new()`, which
panics in the nix build sandbox: with no system CA store,
`ClientBuilder::build()` reaches `rustls_platform_verifier::Verifier::new()`
and fails, and `new()` is `build().expect(..)`. The test passed locally
because a devshell has `/etc/ssl/certs`, and failed in CI.

Disabling certificate verification takes the branch that installs a
no-op verifier and never consults the platform store, so the client
builds anywhere. That is sound in this test and nowhere else: nothing is
sent, the request is built and its bytes are inspected. The helper says
so at the point someone would otherwise object to it.

Found by reading reqwest's `ClientBuilder::build()` rather than trying
builder flags: the first repro attempt — pointing `SSL_CERT_FILE` and
`SSL_CERT_DIR` at nothing — did not reproduce, so any fix verified
against it would have been verified against nothing.
2026-08-17 01:32:50 +02:00
atlas
3d6a97c61d fix(#3363): authenticate the token request with HTTP Basic
Every access-token request the swarm queue client has ever made was
refused. It sent the client id and secret as form fields
(`client_secret_post`); authelia's client registration allows only
`client_secret_basic`, so the identity provider rejected the request
before looking at the credentials at all.

What made it survive so long is the shape of the failure. The refusals
tripped authelia's rate limiter, whose penalty grows faster than this
client's retry interval — 56s, then 296s, then 535s, against a retry
every 60s — so the limiter never drained and a 429 came back before the
credentials were evaluated. The line naming the real cause appeared
roughly once an hour, inside a continuous storm of a different error,
and the storm read as the problem.

Both other callers in this workspace that present client credentials
already use Basic. RFC 6749 says clients SHOULD, authelia's registration
default says so, and a secret in a header is one fewer place for a proxy
to log it.

The test asserts the shape of the request rather than a server's reply:
Authorization is Basic, the body carries the grant type, and neither the
secret nor the client id appears in the body. It fails on the previous
code with no identity provider, no deployment and no network — which is
what this needed and did not have.
2026-08-17 01:32:50 +02:00

View file

@ -296,13 +296,7 @@ async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result<CachedT
source,
})?;
let response = http
.post(&cfg.token_endpoint)
.form(&[
("grant_type", "client_credentials"),
("client_id", cfg.client_id.as_str()),
("client_secret", secret.trim()),
])
let response = token_request(http, cfg, secret.trim())
.send()
.await
.map_err(Error::TokenRequest)?;
@ -321,6 +315,35 @@ async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result<CachedT
})
}
/// Build the token request: `client_credentials`, authenticated with HTTP
/// Basic.
///
/// 🩸 **The credentials go in the `Authorization` header, not the form body.**
/// Both are legal OAuth 2.0 — `client_secret_basic` and `client_secret_post` —
/// but a client registration names *one*, and authelia's default (and ours) is
/// Basic. Sending them in the body got every token request refused with
/// `Client authentication failed … the registered client is configured to only
/// support 'client_secret_basic'`, which reached the operator as an endless
/// `429` because the retries tripped a rate limiter whose penalty grew faster
/// than the retry interval. The 429 then arrived *before* the credentials were
/// ever evaluated, so the one line naming the real cause appeared once an hour.
///
/// RFC 6749 §2.3.1 says clients SHOULD use Basic, both introspection callers in
/// this workspace already do, and a secret in a header is one fewer place for a
/// proxy to log it.
///
/// Split out of [`mint_token`] so the request's *shape* is testable without a
/// running identity provider — see the tests at the bottom of this file.
fn token_request(
http: &reqwest::Client,
cfg: &QueueConfig,
secret: &str,
) -> reqwest::RequestBuilder {
http.post(&cfg.token_endpoint)
.basic_auth(&cfg.client_id, Some(secret))
.form(&[("grant_type", "client_credentials")])
}
/// Build the HTTP client used to reach `cfg.token_endpoint`, trusting
/// `cfg.ca_file` when set. Shared by [`connect`]'s auth callback and by
/// [`mint_token_for`] — anything presenting this identity's credentials to
@ -533,4 +556,85 @@ mod tests {
std::env::remove_var("SWARM_QUEUE_HALF_NATS_URL");
}
}
fn token_cfg() -> QueueConfig {
QueueConfig {
url: "nats://127.0.0.1:4222".to_owned(),
token_endpoint: "https://auth.example.com/api/oidc/token".to_owned(),
client_id: "hive-alpha".to_owned(),
client_secret_file: PathBuf::from("/nonexistent"),
ca_file: None,
}
}
/// 🩸 THE REGRESSION TEST FOR AN OUTAGE THAT RAN FOR WEEKS.
///
/// The credentials used to go in the form body (`client_secret_post`).
/// Authelia's client registration allows only `client_secret_basic`, so
/// every token request was refused — and the refusals tripped a rate
/// limiter whose 429 then arrived *before* the credentials were evaluated,
/// so the error naming the cause appeared roughly once an hour inside a
/// continuous storm of a different error.
///
/// This asserts the *shape of the request* rather than a server's reply,
/// which is the whole point: it fails on the old code with no identity
/// provider, no deployment and no network.
/// A client for inspecting a request, never for sending one.
///
/// 🩸 `reqwest::Client::new()` **panics in the nix build sandbox**, which
/// has no system CA store: `ClientBuilder::build()` reaches
/// `rustls_platform_verifier::Verifier::new()` and fails with "No CA
/// certificates were loaded from the system", and `new()` is
/// `build().expect(..)`. The test passed locally — a devshell has
/// `/etc/ssl/certs` — and failed in CI.
///
/// Turning verification off takes the `!certs_verification` branch, which
/// installs a no-op verifier and never consults the platform store, so
/// this builds anywhere. It is sound *here specifically* because nothing
/// is ever sent: the request is built and its bytes inspected.
fn offline_client() -> reqwest::Client {
reqwest::Client::builder()
.danger_accept_invalid_certs(true)
.build()
.expect("a client that verifies nothing needs no system trust store")
}
#[test]
fn the_token_request_authenticates_with_http_basic() {
let req = token_request(&offline_client(), &token_cfg(), "s3cret")
.build()
.expect("the token request must build");
let auth = req
.headers()
.get(reqwest::header::AUTHORIZATION)
.expect("credentials must travel in the Authorization header")
.to_str()
.expect("the header is ascii");
assert!(
auth.starts_with("Basic "),
"must be client_secret_basic, got: {auth}"
);
let body = std::str::from_utf8(
req.body()
.and_then(reqwest::Body::as_bytes)
.expect("the request has an in-memory body"),
)
.expect("the body is utf-8");
assert!(
body.contains("grant_type=client_credentials"),
"the grant type still belongs in the body, got: {body}"
);
// The half that was actually broken: a secret in the body is both the
// wrong auth method for our registration and a value proxies log.
assert!(
!body.contains("client_secret"),
"the secret must not be in the request body, got: {body}"
);
assert!(
!body.contains("client_id"),
"the client id belongs in the Basic credentials, got: {body}"
);
}
}