fix(security): bind Foundry API keys to request origin (#11347)
Signed-off-by: Jasper Hugo <jasper@spiral.xyz>
This commit is contained in:
@@ -34,11 +34,12 @@ pub struct ApiClient {
|
||||
transport_policy: TransportPolicy,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
#[derive(Clone)]
|
||||
enum TransportPolicy {
|
||||
Default,
|
||||
HttpsOnly,
|
||||
LoopbackHttp,
|
||||
SameOrigin(url::Origin),
|
||||
}
|
||||
|
||||
pub enum AuthMethod {
|
||||
@@ -313,7 +314,7 @@ impl ApiClient {
|
||||
fn rebuild_client(&mut self) -> Result<()> {
|
||||
let mut client_builder =
|
||||
Self::client_builder(self.timeout).default_headers(self.default_headers.clone());
|
||||
client_builder = Self::configure_transport(client_builder, self.transport_policy);
|
||||
client_builder = Self::configure_transport(client_builder, &self.transport_policy);
|
||||
|
||||
// Configure TLS if needed
|
||||
if let Some(ref tls_config) = self.tls_config {
|
||||
@@ -326,7 +327,7 @@ impl ApiClient {
|
||||
|
||||
fn configure_transport(
|
||||
client_builder: reqwest::ClientBuilder,
|
||||
transport_policy: TransportPolicy,
|
||||
transport_policy: &TransportPolicy,
|
||||
) -> reqwest::ClientBuilder {
|
||||
match transport_policy {
|
||||
TransportPolicy::Default => client_builder,
|
||||
@@ -353,6 +354,19 @@ impl ApiClient {
|
||||
}
|
||||
}))
|
||||
}
|
||||
TransportPolicy::SameOrigin(origin) => {
|
||||
let origin = origin.clone();
|
||||
client_builder.redirect(Policy::custom(move |attempt| {
|
||||
if attempt.previous().len() >= 10 {
|
||||
return attempt.error("too many redirects");
|
||||
}
|
||||
if attempt.url().origin() == origin {
|
||||
attempt.follow()
|
||||
} else {
|
||||
attempt.error("redirect crosses the authenticated request origin")
|
||||
}
|
||||
}))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -427,6 +441,15 @@ impl ApiClient {
|
||||
Ok(self)
|
||||
}
|
||||
|
||||
pub fn with_same_origin_redirects(mut self) -> Result<Self> {
|
||||
let origin = url::Url::parse(&self.host)
|
||||
.map_err(|error| anyhow::anyhow!("Invalid base URL: {}", error))?
|
||||
.origin();
|
||||
self.transport_policy = TransportPolicy::SameOrigin(origin);
|
||||
self.rebuild_client()?;
|
||||
Ok(self)
|
||||
}
|
||||
|
||||
pub fn request<'a>(&'a self, path: &'a str) -> ApiRequestBuilder<'a> {
|
||||
ApiRequestBuilder {
|
||||
client: self,
|
||||
|
||||
@@ -374,7 +374,11 @@ fn configured_client(
|
||||
tls_config: Option<TlsConfig>,
|
||||
request_builder: Option<RequestBuilderDecorator>,
|
||||
) -> Result<ApiClient> {
|
||||
let restrict_redirects = matches!(&auth, AuthMethod::ApiKey { .. });
|
||||
let mut client = ApiClient::new_with_tls(host, auth, tls_config)?;
|
||||
if restrict_redirects {
|
||||
client = client.with_same_origin_redirects()?;
|
||||
}
|
||||
if let Some(request_builder) = request_builder {
|
||||
client = client.with_request_builder(request_builder);
|
||||
}
|
||||
@@ -547,7 +551,7 @@ impl Provider for AzureFoundryProvider {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use serde_json::json;
|
||||
use wiremock::matchers::{body_partial_json, method, path, query_param};
|
||||
use wiremock::matchers::{body_partial_json, header, method, path, query_param};
|
||||
use wiremock::{Mock, MockServer, ResponseTemplate};
|
||||
|
||||
fn project_endpoint(server: &MockServer) -> String {
|
||||
@@ -733,6 +737,123 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn foundry_api_keys_do_not_follow_cross_origin_redirects() {
|
||||
for header_name in ["api-key", "x-api-key"] {
|
||||
let destination = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/capture"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.mount(&destination)
|
||||
.await;
|
||||
|
||||
let source = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/redirect"))
|
||||
.and(header(header_name, "foundry-secret"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(307)
|
||||
.append_header("location", format!("{}/capture", destination.uri())),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&source)
|
||||
.await;
|
||||
|
||||
let client = configured_client(
|
||||
source.uri(),
|
||||
AuthMethod::ApiKey {
|
||||
header_name: header_name.to_string(),
|
||||
key: "foundry-secret".to_string(),
|
||||
},
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(client.response_get("redirect").await.is_err());
|
||||
assert!(destination.received_requests().await.unwrap().is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn foundry_api_keys_follow_same_origin_redirects() {
|
||||
for header_name in ["api-key", "x-api-key"] {
|
||||
let server = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/redirect"))
|
||||
.respond_with(ResponseTemplate::new(307).append_header("location", "/final"))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/final"))
|
||||
.and(header(header_name, "foundry-secret"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
let client = configured_client(
|
||||
server.uri(),
|
||||
AuthMethod::ApiKey {
|
||||
header_name: header_name.to_string(),
|
||||
key: "foundry-secret".to_string(),
|
||||
},
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(client
|
||||
.response_get("redirect")
|
||||
.await
|
||||
.unwrap()
|
||||
.status()
|
||||
.is_success());
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn foundry_bearer_redirects_keep_reqwest_authorization_behavior() {
|
||||
let destination = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/capture"))
|
||||
.respond_with(ResponseTemplate::new(200))
|
||||
.expect(1)
|
||||
.mount(&destination)
|
||||
.await;
|
||||
|
||||
let source = MockServer::start().await;
|
||||
Mock::given(method("GET"))
|
||||
.and(path("/redirect"))
|
||||
.and(header("authorization", "Bearer foundry-token"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(307)
|
||||
.append_header("location", format!("{}/capture", destination.uri())),
|
||||
)
|
||||
.expect(1)
|
||||
.mount(&source)
|
||||
.await;
|
||||
|
||||
let client = configured_client(
|
||||
source.uri(),
|
||||
AuthMethod::BearerToken("foundry-token".to_string()),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert!(client
|
||||
.response_get("redirect")
|
||||
.await
|
||||
.unwrap()
|
||||
.status()
|
||||
.is_success());
|
||||
let requests = destination.received_requests().await.unwrap();
|
||||
assert_eq!(requests.len(), 1);
|
||||
assert!(requests[0].headers.get("authorization").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deployment_metadata_enriches_context_without_pricing() {
|
||||
let info = model_info_for_deployment("production-chat", "gpt-5");
|
||||
|
||||
@@ -24,6 +24,20 @@ struct AzureFoundryAuthProvider {
|
||||
header: AuthHeader,
|
||||
}
|
||||
|
||||
fn auth_method(auth: &Arc<AzureAuth>, header: AuthHeader) -> AuthMethod {
|
||||
match (auth.credential_type(), header) {
|
||||
(AzureCredentials::ApiKey(key), AuthHeader::ApiKey) => AuthMethod::ApiKey {
|
||||
header_name: "api-key".to_string(),
|
||||
key: key.clone(),
|
||||
},
|
||||
(AzureCredentials::ApiKey(key), AuthHeader::Bearer) => AuthMethod::BearerToken(key.clone()),
|
||||
(_, header) => AuthMethod::Custom(Box::new(AzureFoundryAuthProvider {
|
||||
auth: Arc::clone(auth),
|
||||
header,
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl AuthProvider for AzureFoundryAuthProvider {
|
||||
async fn get_auth_header(&self) -> Result<(String, String)> {
|
||||
@@ -89,18 +103,12 @@ pub async fn from_env(tls_config: Option<TlsConfig>) -> Result<AzureFoundryProvi
|
||||
ad_token,
|
||||
resource.to_string(),
|
||||
)?);
|
||||
let auth_method = |header| {
|
||||
AuthMethod::Custom(Box::new(AzureFoundryAuthProvider {
|
||||
auth: Arc::clone(&auth),
|
||||
header,
|
||||
}))
|
||||
};
|
||||
let anthropic_auth = match auth.credential_type() {
|
||||
AzureCredentials::ApiKey(key) => AuthMethod::ApiKey {
|
||||
header_name: "x-api-key".to_string(),
|
||||
key: key.clone(),
|
||||
},
|
||||
_ => auth_method(AuthHeader::Bearer),
|
||||
_ => auth_method(&auth, AuthHeader::Bearer),
|
||||
};
|
||||
let api_key_auth_header = || match auth.credential_type() {
|
||||
AzureCredentials::ApiKey(_) => AuthHeader::ApiKey,
|
||||
@@ -115,10 +123,10 @@ pub async fn from_env(tls_config: Option<TlsConfig>) -> Result<AzureFoundryProvi
|
||||
endpoint,
|
||||
api_version,
|
||||
maas_model,
|
||||
auth_method(chat_auth_header),
|
||||
auth_method(api_key_auth_header()),
|
||||
auth_method(&auth, chat_auth_header),
|
||||
auth_method(&auth, api_key_auth_header()),
|
||||
anthropic_auth,
|
||||
auth_method(api_key_auth_header()),
|
||||
auth_method(&auth, api_key_auth_header()),
|
||||
tls_config,
|
||||
Some(crate::session_context::session_id_request_builder()),
|
||||
)
|
||||
@@ -128,39 +136,48 @@ pub async fn from_env(tls_config: Option<TlsConfig>) -> Result<AzureFoundryProvi
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
async fn header(
|
||||
api_key: Option<&str>,
|
||||
ad_token: Option<&str>,
|
||||
header: AuthHeader,
|
||||
) -> (String, String) {
|
||||
let auth = Arc::new(
|
||||
fn auth(api_key: Option<&str>, ad_token: Option<&str>) -> Arc<AzureAuth> {
|
||||
Arc::new(
|
||||
AzureAuth::new_with_resource(
|
||||
api_key.map(str::to_string),
|
||||
ad_token.map(str::to_string),
|
||||
AZURE_PROJECT_ENTRA_RESOURCE.to_string(),
|
||||
)
|
||||
.unwrap(),
|
||||
);
|
||||
AzureFoundryAuthProvider { auth, header }
|
||||
.get_auth_header()
|
||||
.await
|
||||
.unwrap()
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn project_api_key_uses_api_key_header() {
|
||||
assert_eq!(
|
||||
header(Some("key"), None, AuthHeader::ApiKey).await,
|
||||
("api-key".to_string(), "key".to_string())
|
||||
);
|
||||
async fn header(
|
||||
api_key: Option<&str>,
|
||||
ad_token: Option<&str>,
|
||||
header: AuthHeader,
|
||||
) -> (String, String) {
|
||||
AzureFoundryAuthProvider {
|
||||
auth: auth(api_key, ad_token),
|
||||
header,
|
||||
}
|
||||
.get_auth_header()
|
||||
.await
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn maas_api_key_uses_bearer_header() {
|
||||
assert_eq!(
|
||||
header(Some("key"), None, AuthHeader::Bearer).await,
|
||||
("Authorization".to_string(), "Bearer key".to_string())
|
||||
);
|
||||
#[test]
|
||||
fn project_api_key_uses_origin_bound_auth_method() {
|
||||
match auth_method(&auth(Some("key"), None), AuthHeader::ApiKey) {
|
||||
AuthMethod::ApiKey { header_name, key } => {
|
||||
assert_eq!(header_name, "api-key");
|
||||
assert_eq!(key, "key");
|
||||
}
|
||||
_ => panic!("project API keys must use the origin-bound API key auth method"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn maas_api_key_uses_standard_bearer_auth_method() {
|
||||
match auth_method(&auth(Some("key"), None), AuthHeader::Bearer) {
|
||||
AuthMethod::BearerToken(token) => assert_eq!(token, "key"),
|
||||
_ => panic!("MaaS API keys must use the standard bearer auth method"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user