mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Default gateway OAuth delivery to Authorization bearer tokens (#57313)
## What changed Allow omitting `gateway_oauth.delivery` to send the gateway token as `Authorization: Bearer …`. Custom headers and cookies remain available when a provider also needs its own credentials. Reject conflicting provider authentication and duplicate delivery headers with actionable errors. For gateway bearer delivery, require `model_catalog_url` to share the scheme, host, and port of `base_url`. Consistently reject request header collisions across auth application methods. ## Testing Add coverage for omitted delivery, provider credential conflicts, and model catalog origin validation. Update inference tests to verify gateway-only authentication while signed in and combined ChatGPT and gateway credentials in separate headers. GitOrigin-RevId: 8564d3998511b3f6c55f3b65a01ed9056a90417e
This commit is contained in:
@@ -1968,7 +1968,17 @@
|
||||
"type": "string"
|
||||
},
|
||||
"delivery": {
|
||||
"$ref": "#/definitions/GatewayOAuthDelivery"
|
||||
"allOf": [
|
||||
{
|
||||
"$ref": "#/definitions/GatewayOAuthDelivery"
|
||||
}
|
||||
],
|
||||
"default": {
|
||||
"kind": "header",
|
||||
"name": "Authorization",
|
||||
"scheme": "Bearer"
|
||||
},
|
||||
"description": "Omit to send the gateway token via the Authorization header"
|
||||
},
|
||||
"redirect_port": {
|
||||
"format": "uint16",
|
||||
@@ -1992,7 +2002,6 @@
|
||||
"required": [
|
||||
"authorization_url",
|
||||
"client_id",
|
||||
"delivery",
|
||||
"token_url"
|
||||
],
|
||||
"type": "object"
|
||||
|
||||
@@ -12,13 +12,12 @@ use codex_model_provider::AgentIdentitySessionFallback;
|
||||
use codex_model_provider::ProviderAuthScope;
|
||||
use codex_model_provider::create_model_provider;
|
||||
use codex_model_provider::test_support::seed_gateway_auth;
|
||||
use codex_model_provider_info::GatewayOAuthConfig;
|
||||
use codex_model_provider_info::GatewayOAuthDelivery;
|
||||
use codex_model_provider_info::ModelProviderInfo;
|
||||
use codex_protocol::protocol::EventMsg;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -31,20 +30,56 @@ use wiremock::matchers::method;
|
||||
use wiremock::matchers::path;
|
||||
|
||||
#[tokio::test]
|
||||
async fn sampling_uses_gateway_and_primary_auth() -> Result<()> {
|
||||
sampling_requires_gateway_and_primary_auth(/*expected_error*/ None).await
|
||||
async fn inference_sends_chatgpt_and_gateway_credentials_to_the_configured_headers() -> Result<()> {
|
||||
sample_with_gateway(
|
||||
"requires_openai_auth = true",
|
||||
"delivery = { kind = 'header', name = 'x-gateway-auth' }",
|
||||
CodexAuth::from_external_chatgpt_tokens(
|
||||
"header.e30.sig",
|
||||
"test-account",
|
||||
/*chatgpt_plan_type*/ None,
|
||||
)?,
|
||||
&[
|
||||
("authorization", "Bearer header.e30.sig"),
|
||||
("x-gateway-auth", "Bearer gateway-token"),
|
||||
("chatgpt-account-id", "test-account"),
|
||||
],
|
||||
/*expected_error*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn gateway_token_failure_blocks_sampling() -> Result<()> {
|
||||
sampling_requires_gateway_and_primary_auth(Some(
|
||||
"Gateway OAuth authentication failed; check the gateway configuration and credential store.",
|
||||
)).await
|
||||
sample_with_gateway(
|
||||
"requires_openai_auth = true",
|
||||
"delivery = { kind = 'header', name = 'x-gateway-auth' }",
|
||||
CodexAuth::from_api_key("provider-token"),
|
||||
&[],
|
||||
Some("Gateway OAuth authentication failed; check the gateway configuration and credential store."),
|
||||
).await
|
||||
}
|
||||
|
||||
async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str>) -> Result<()> {
|
||||
#[tokio::test]
|
||||
async fn omitted_delivery_sends_only_gateway_auth_even_when_signed_in() -> Result<()> {
|
||||
sample_with_gateway(
|
||||
"requires_openai_auth = false",
|
||||
"",
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing(),
|
||||
&[("authorization", "Bearer gateway-token")],
|
||||
/*expected_error*/ None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn sample_with_gateway(
|
||||
provider_settings: &str,
|
||||
gateway_settings: &str,
|
||||
primary_auth: CodexAuth,
|
||||
expected_headers: &[(&str, &str)],
|
||||
expected_error: Option<&str>,
|
||||
) -> Result<()> {
|
||||
let server = MockServer::start().await;
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
let response = if expected_error.is_none() {
|
||||
Some(
|
||||
responses::mount_sse_once(
|
||||
@@ -66,38 +101,94 @@ async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str>
|
||||
.await;
|
||||
None
|
||||
};
|
||||
let mut provider =
|
||||
ModelProviderInfo::create_openai_provider(Some(format!("{}/v1", server.uri())));
|
||||
provider.supports_websockets = false;
|
||||
provider.gateway_oauth = Some(GatewayOAuthConfig {
|
||||
authorization_url: format!("{}/authorize", server.uri()),
|
||||
token_url: format!("{}/token", server.uri()),
|
||||
client_id: "client".into(),
|
||||
resource: None,
|
||||
scopes: vec![],
|
||||
redirect_port: None,
|
||||
delivery: GatewayOAuthDelivery::Header {
|
||||
name: "x-gateway-auth".into(),
|
||||
scheme: "Bearer".into(),
|
||||
},
|
||||
});
|
||||
let expires_at = if expected_error.is_some() {
|
||||
0
|
||||
} else {
|
||||
i64::MAX
|
||||
};
|
||||
let test = gateway_codex(
|
||||
&server,
|
||||
provider_settings,
|
||||
gateway_settings,
|
||||
primary_auth,
|
||||
expires_at,
|
||||
)
|
||||
.await?;
|
||||
assert_eq!(run_gateway_turn(&test).await?.as_deref(), expected_error);
|
||||
if let Some(response) = response {
|
||||
let request = response.single_request();
|
||||
for name in [
|
||||
"authorization",
|
||||
"x-gateway-auth",
|
||||
"cookie",
|
||||
"chatgpt-account-id",
|
||||
] {
|
||||
let expected = expected_headers
|
||||
.iter()
|
||||
.find(|(key, _)| *key == name)
|
||||
.map(|(_, value)| *value);
|
||||
assert_eq!(
|
||||
request.header(name).as_deref(),
|
||||
expected,
|
||||
"{gateway_settings}: {name}"
|
||||
);
|
||||
}
|
||||
} else {
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("recorded gateway requests");
|
||||
assert!(!requests.is_empty());
|
||||
assert!(
|
||||
requests
|
||||
.iter()
|
||||
.all(|request| request.url.path() == "/token")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn gateway_codex(
|
||||
server: &MockServer,
|
||||
provider_settings: &str,
|
||||
gateway_settings: &str,
|
||||
primary_auth: CodexAuth,
|
||||
expires_at: i64,
|
||||
) -> Result<TestCodex> {
|
||||
let home = Arc::new(TempDir::new()?);
|
||||
let provider: ModelProviderInfo = toml::from_str(&format!(
|
||||
r#"
|
||||
name = "gateway"
|
||||
base_url = "{base}/v1"
|
||||
{provider_settings}
|
||||
[gateway_oauth]
|
||||
authorization_url = "{base}/authorize"
|
||||
token_url = "{base}/token"
|
||||
client_id = "client"
|
||||
{gateway_settings}
|
||||
"#,
|
||||
base = server.uri()
|
||||
))?;
|
||||
let primary = AuthManager::from_auth_for_testing_with_home(
|
||||
CodexAuth::from_api_key("primary-token"),
|
||||
primary_auth.clone(),
|
||||
home.path().to_path_buf(),
|
||||
);
|
||||
let _gateway = seed_gateway_auth(
|
||||
&provider,
|
||||
&primary,
|
||||
json!({"access_token": "gateway-token", "refresh_token": "refresh-token", "expires_at": if expected_error.is_some() { 0 } else { i64::MAX }}),
|
||||
json!({"access_token": "gateway-token", "refresh_token": "refresh-token", "expires_at": expires_at}),
|
||||
);
|
||||
let test = test_codex()
|
||||
test_codex()
|
||||
.with_home(home)
|
||||
.with_auth(CodexAuth::from_api_key("primary-token"))
|
||||
.with_auth(primary_auth)
|
||||
.with_config(move |config| {
|
||||
config.model_provider = provider;
|
||||
})
|
||||
.build_with_auto_env(&server)
|
||||
.await?;
|
||||
.build_with_auto_env(server)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn run_gateway_turn(test: &TestCodex) -> Result<Option<String>> {
|
||||
test.codex
|
||||
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
@@ -112,32 +203,7 @@ async fn sampling_requires_gateway_and_primary_auth(expected_error: Option<&str>
|
||||
matches!(event, EventMsg::TurnComplete(_))
|
||||
})
|
||||
.await;
|
||||
assert_eq!(error_message.as_deref(), expected_error);
|
||||
if let Some(response) = response {
|
||||
let request = response.single_request();
|
||||
assert_eq!(
|
||||
(
|
||||
request.header("authorization"),
|
||||
request.header("x-gateway-auth")
|
||||
),
|
||||
(
|
||||
Some("Bearer primary-token".to_string()),
|
||||
Some("Bearer gateway-token".to_string())
|
||||
),
|
||||
);
|
||||
} else {
|
||||
let requests = server
|
||||
.received_requests()
|
||||
.await
|
||||
.expect("recorded gateway requests");
|
||||
assert!(!requests.is_empty());
|
||||
assert!(
|
||||
requests
|
||||
.iter()
|
||||
.all(|request| request.url.path() == "/token")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
Ok(error_message)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -36,6 +36,8 @@ pub struct GatewayOAuthConfig {
|
||||
#[serde(default)]
|
||||
pub scopes: Vec<String>,
|
||||
pub redirect_port: Option<u16>,
|
||||
/// Omit to send the gateway token via the Authorization header
|
||||
#[serde(default)]
|
||||
pub delivery: GatewayOAuthDelivery,
|
||||
}
|
||||
|
||||
@@ -60,11 +62,55 @@ pub enum GatewayOAuthDelivery {
|
||||
},
|
||||
}
|
||||
|
||||
impl Default for GatewayOAuthDelivery {
|
||||
fn default() -> Self {
|
||||
Self::Header {
|
||||
name: "Authorization".into(),
|
||||
scheme: bearer_scheme(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl GatewayOAuthDelivery {
|
||||
fn is_authorization_bearer(&self) -> bool {
|
||||
matches!(self, Self::Header { name, scheme } if name.eq_ignore_ascii_case("authorization") && scheme.eq_ignore_ascii_case("bearer"))
|
||||
}
|
||||
|
||||
fn secondary_header(&self) -> Result<HeaderName, &'static str> {
|
||||
match self {
|
||||
Self::Header { name, scheme } => {
|
||||
let header = HeaderName::from_bytes(name.as_bytes())
|
||||
.map_err(|_| "needs a valid HTTP header name, such as X-Gateway-Auth")?;
|
||||
if RESERVED_GATEWAY_OAUTH_HEADERS.contains(&header.as_str())
|
||||
|| RESERVED_GATEWAY_OAUTH_HEADER_PREFIXES
|
||||
.iter()
|
||||
.any(|prefix| header.as_str().starts_with(prefix))
|
||||
{
|
||||
return Err(
|
||||
"uses a reserved header. Choose a custom header or cookie accepted by your gateway",
|
||||
);
|
||||
}
|
||||
if !is_token(scheme) {
|
||||
return Err("needs an HTTP authentication scheme such as Bearer");
|
||||
}
|
||||
Ok(header)
|
||||
}
|
||||
Self::Cookie { name } if is_token(name) => Ok(http::header::COOKIE),
|
||||
Self::Cookie { .. } => Err("needs a valid cookie name, such as gateway_session"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn bearer_scheme() -> String {
|
||||
"Bearer".to_owned()
|
||||
}
|
||||
|
||||
impl GatewayOAuthConfig {
|
||||
/// Send the gateway bearer only to the configured endpoint.
|
||||
pub fn requires_gateway_origin(&self) -> bool {
|
||||
self.delivery.is_authorization_bearer()
|
||||
}
|
||||
|
||||
pub(crate) fn validate(&self, provider: &ModelProviderInfo) -> Result<(), String> {
|
||||
if provider.aws.is_some() || provider.is_amazon_bedrock() {
|
||||
return Err("provider gateway_oauth cannot be combined with AWS authentication".into());
|
||||
@@ -75,34 +121,44 @@ impl GatewayOAuthConfig {
|
||||
.base_url
|
||||
.as_deref()
|
||||
.ok_or("gateway_oauth requires base_url")?;
|
||||
validate_endpoint_url(base_url, "base_url")?;
|
||||
let gateway = validate_endpoint_url(base_url, "base_url")?;
|
||||
if self.requires_gateway_origin()
|
||||
&& let Some(catalog) = provider.model_catalog_url.as_deref()
|
||||
&& validate_endpoint_url(catalog, "model_catalog_url")?.origin() != gateway.origin()
|
||||
{
|
||||
return Err(
|
||||
"Codex would send gateway credentials to a different model catalog. Set model_catalog_url \
|
||||
to a URL with the same scheme, host, and port as base_url, or remove model_catalog_url \
|
||||
to use the gateway's catalog."
|
||||
.into(),
|
||||
);
|
||||
}
|
||||
if self.client_id.trim().is_empty() || self.redirect_port == Some(0) {
|
||||
return Err(
|
||||
"gateway_oauth requires a nonempty client_id and a nonzero redirect_port".into(),
|
||||
);
|
||||
}
|
||||
let header = match &self.delivery {
|
||||
GatewayOAuthDelivery::Header { name, scheme } => {
|
||||
let header = HeaderName::from_bytes(name.as_bytes())
|
||||
.map_err(|_| "invalid gateway_oauth header name")?;
|
||||
if !is_token(scheme)
|
||||
|| RESERVED_GATEWAY_OAUTH_HEADERS.contains(&header.as_str())
|
||||
|| RESERVED_GATEWAY_OAUTH_HEADER_PREFIXES
|
||||
.iter()
|
||||
.any(|prefix| header.as_str().starts_with(prefix))
|
||||
{
|
||||
return Err(
|
||||
"invalid or reserved gateway_oauth delivery header or scheme".into(),
|
||||
);
|
||||
}
|
||||
header
|
||||
let gateway_header = if self.delivery.is_authorization_bearer() {
|
||||
if provider.is_gcp() {
|
||||
return Err("Google Cloud uses the Authorization header for its own token. Remove gateway_oauth to use the built-in Google Cloud provider".into());
|
||||
}
|
||||
GatewayOAuthDelivery::Cookie { name } => {
|
||||
if !is_token(name) {
|
||||
return Err("invalid gateway_oauth cookie name".into());
|
||||
}
|
||||
http::header::COOKIE
|
||||
if provider.requires_openai_auth
|
||||
|| provider.env_key.is_some()
|
||||
|| provider.experimental_bearer_token.is_some()
|
||||
|| provider.has_command_auth()
|
||||
{
|
||||
return Err(
|
||||
"Header conflict. Codex is configured to send both the model provider credential \
|
||||
and the gateway token in the Authorization header. Set gateway_oauth.delivery \
|
||||
to a different header or cookie that your gateway accepts for its token."
|
||||
.into(),
|
||||
);
|
||||
}
|
||||
http::header::AUTHORIZATION
|
||||
} else {
|
||||
self.delivery
|
||||
.secondary_header()
|
||||
.map_err(|reason| format!("gateway_oauth.delivery {reason}"))?
|
||||
};
|
||||
if provider
|
||||
.http_headers
|
||||
@@ -114,11 +170,11 @@ impl GatewayOAuthConfig {
|
||||
.iter()
|
||||
.flat_map(|headers| headers.keys()),
|
||||
)
|
||||
.any(|name| name.eq_ignore_ascii_case(header.as_str()))
|
||||
.any(|name| name.eq_ignore_ascii_case(gateway_header.as_str()))
|
||||
{
|
||||
return Err(
|
||||
"gateway_oauth delivery conflicts with a configured provider header".into(),
|
||||
);
|
||||
return Err(format!(
|
||||
"The {gateway_header} header is set in both gateway_oauth.delivery and the provider's http_headers or env_http_headers. Remove the duplicate header from the provider or choose a different gateway delivery."
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -52,7 +52,6 @@ fn rejects_unsafe_endpoints_and_delivery_collisions() {
|
||||
assert!(!error.contains(url));
|
||||
}
|
||||
for name in [
|
||||
"Authorization",
|
||||
"HOST",
|
||||
"x-openai-actor-authorization",
|
||||
"content-length",
|
||||
@@ -77,3 +76,45 @@ fn rejects_unsafe_endpoints_and_delivery_collisions() {
|
||||
)]));
|
||||
assert!(cookie.validate().is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gateway_bearer_requires_the_model_catalog_to_share_its_origin() {
|
||||
let other = "https://gateway.example.test:8443/catalog";
|
||||
let mut info = provider("");
|
||||
info.model_catalog_url = Some(other.into());
|
||||
assert!(
|
||||
info.validate()
|
||||
.unwrap_err()
|
||||
.contains("same scheme, host, and port as base_url")
|
||||
);
|
||||
|
||||
info.model_catalog_url = Some("https://gateway.example.test:443/custom/catalog".into());
|
||||
assert_eq!(info.validate(), Ok(()));
|
||||
|
||||
let mut legacy = provider("delivery = { kind = 'header', name = 'x-gateway-auth' }");
|
||||
legacy.model_catalog_url = Some(other.into());
|
||||
assert_eq!(legacy.validate(), Ok(()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn omitted_delivery_rejects_an_existing_provider_bearer() {
|
||||
let mut info = provider("");
|
||||
assert_eq!(info.validate(), Ok(()));
|
||||
info.requires_openai_auth = true;
|
||||
assert!(
|
||||
info.validate()
|
||||
.unwrap_err()
|
||||
.contains("gateway_oauth.delivery")
|
||||
);
|
||||
info.requires_openai_auth = false;
|
||||
info.env_key = Some("PROVIDER_API_KEY".into());
|
||||
assert!(
|
||||
info.validate()
|
||||
.unwrap_err()
|
||||
.contains("gateway_oauth.delivery")
|
||||
);
|
||||
|
||||
let mut explicit = provider("delivery = { kind = 'header', name = 'x-gateway-auth' }");
|
||||
explicit.env_key = info.env_key;
|
||||
assert_eq!(explicit.validate(), Ok(()));
|
||||
}
|
||||
|
||||
@@ -63,9 +63,9 @@ pub(crate) async fn compose_auth(
|
||||
.map_err(|error| codex_api::map_api_error(error.into()))?
|
||||
.contains_key(&name)
|
||||
{
|
||||
return Err(CodexErr::InvalidRequest(
|
||||
"gateway OAuth conflicts with primary auth headers".into(),
|
||||
));
|
||||
return Err(CodexErr::InvalidRequest(format!(
|
||||
"The model provider already sets the {name} header. Choose a different header or cookie in gateway_oauth.delivery for the gateway token."
|
||||
)));
|
||||
}
|
||||
resolved.auth = Arc::new(CombinedAuth {
|
||||
primary: resolved.auth,
|
||||
@@ -111,26 +111,29 @@ struct CombinedAuth {
|
||||
value: HeaderValue,
|
||||
}
|
||||
|
||||
impl CombinedAuth {
|
||||
fn insert_gateway_header(&self, headers: &mut HeaderMap) -> std::result::Result<(), AuthError> {
|
||||
if headers.contains_key(&self.name) {
|
||||
return Err(AuthError::Build(format!(
|
||||
"The request already sets the {} header. Choose a different header or cookie in gateway_oauth.delivery for the gateway token.",
|
||||
self.name
|
||||
)));
|
||||
}
|
||||
headers.insert(self.name.clone(), self.value.clone());
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl AuthProvider for CombinedAuth {
|
||||
fn add_auth_headers(&self, headers: &mut HeaderMap) -> std::result::Result<(), AuthError> {
|
||||
self.primary.add_auth_headers(headers)?;
|
||||
headers
|
||||
.entry(self.name.clone())
|
||||
.or_insert(self.value.clone());
|
||||
Ok(())
|
||||
self.insert_gateway_header(headers)
|
||||
}
|
||||
|
||||
fn apply_auth(&self, request: codex_http_client::Request) -> AuthProviderFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let mut request = self.primary.apply_auth(request).await?;
|
||||
if request.headers.contains_key(&self.name) {
|
||||
return Err(AuthError::Build(
|
||||
"gateway OAuth conflicts with request headers".into(),
|
||||
));
|
||||
}
|
||||
request
|
||||
.headers
|
||||
.insert(self.name.clone(), self.value.clone());
|
||||
self.insert_gateway_header(&mut request.headers)?;
|
||||
Ok(request)
|
||||
})
|
||||
}
|
||||
@@ -138,12 +141,7 @@ impl AuthProvider for CombinedAuth {
|
||||
fn apply_auth_headers(&self, headers: HeaderMap) -> AuthHeadersFuture<'_> {
|
||||
Box::pin(async move {
|
||||
let mut headers = self.primary.apply_auth_headers(headers).await?;
|
||||
if headers.contains_key(&self.name) {
|
||||
return Err(AuthError::Build(
|
||||
"gateway OAuth conflicts with request headers".into(),
|
||||
));
|
||||
}
|
||||
headers.insert(self.name.clone(), self.value.clone());
|
||||
self.insert_gateway_header(&mut headers)?;
|
||||
Ok(headers)
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user