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.
This commit is contained in:
parent
22068bd2b1
commit
3d6a97c61d
1 changed files with 91 additions and 7 deletions
|
|
@ -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,65 @@ 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.
|
||||
#[test]
|
||||
fn the_token_request_authenticates_with_http_basic() {
|
||||
let req = token_request(&reqwest::Client::new(), &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}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue