Compare commits
1 changed files with 7 additions and 111 deletions
|
|
@ -296,7 +296,13 @@ async fn mint_token(http: &reqwest::Client, cfg: &QueueConfig) -> Result<CachedT
|
||||||
source,
|
source,
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let response = token_request(http, cfg, secret.trim())
|
let response = http
|
||||||
|
.post(&cfg.token_endpoint)
|
||||||
|
.form(&[
|
||||||
|
("grant_type", "client_credentials"),
|
||||||
|
("client_id", cfg.client_id.as_str()),
|
||||||
|
("client_secret", secret.trim()),
|
||||||
|
])
|
||||||
.send()
|
.send()
|
||||||
.await
|
.await
|
||||||
.map_err(Error::TokenRequest)?;
|
.map_err(Error::TokenRequest)?;
|
||||||
|
|
@ -315,35 +321,6 @@ 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
|
/// Build the HTTP client used to reach `cfg.token_endpoint`, trusting
|
||||||
/// `cfg.ca_file` when set. Shared by [`connect`]'s auth callback and by
|
/// `cfg.ca_file` when set. Shared by [`connect`]'s auth callback and by
|
||||||
/// [`mint_token_for`] — anything presenting this identity's credentials to
|
/// [`mint_token_for`] — anything presenting this identity's credentials to
|
||||||
|
|
@ -556,85 +533,4 @@ mod tests {
|
||||||
std::env::remove_var("SWARM_QUEUE_HALF_NATS_URL");
|
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}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue