mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
Fix OpenAI Codex OAuth device auth
Align the OpenAI device-auth flow with the live Codex endpoints and allow API-backed OpenAI resolution to fall back to the stored openai_codex credential. This makes provider login work against the current OpenAI response shape and lets doctor/workflows use the saved credential without OPENAI_API_KEY in the environment.
This commit is contained in:
parent
56de288f59
commit
6e80d8b8f2
3 changed files with 347 additions and 42 deletions
|
|
@ -272,7 +272,7 @@ fn credential_ids_for(provider: Provider, usage: CredentialUsage) -> &'static [&
|
|||
(Provider::OpenAi, CredentialUsage::CliAgent(CliAgentKind::Codex)) => {
|
||||
&["openai_codex", "openai"]
|
||||
}
|
||||
(Provider::OpenAi, _) => &["openai"],
|
||||
(Provider::OpenAi, _) => &["openai", "openai_codex"],
|
||||
(Provider::Anthropic, _) => &["anthropic"],
|
||||
(Provider::Gemini, _) => &["gemini"],
|
||||
(Provider::Kimi, _) => &["kimi"],
|
||||
|
|
@ -357,6 +357,40 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_openai_api_request_falls_back_to_codex_oauth_credential() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut vault = Vault::load(dir.path().join("secrets.json")).unwrap();
|
||||
vault_set_credential(
|
||||
&mut vault,
|
||||
"openai_codex",
|
||||
&oauth_credential(
|
||||
"https://auth.openai.com/oauth/token".to_string(),
|
||||
Utc::now() + Duration::hours(1),
|
||||
),
|
||||
)
|
||||
.unwrap();
|
||||
let resolver = test_resolver(vault, Arc::new(|_| None));
|
||||
|
||||
let resolved = resolver
|
||||
.resolve(Provider::OpenAi, CredentialUsage::ApiRequest)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let ResolvedCredential::Api(api) = resolved else {
|
||||
panic!("expected api credential");
|
||||
};
|
||||
assert_eq!(
|
||||
api.auth_header,
|
||||
ApiKeyHeader::Bearer("expired-access".to_string())
|
||||
);
|
||||
assert!(api.codex_mode);
|
||||
assert_eq!(
|
||||
api.base_url.as_deref(),
|
||||
Some("https://chatgpt.com/backend-api/codex")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn resolve_returns_not_configured_for_missing_provider() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ use std::time::{Duration, Instant};
|
|||
use async_trait::async_trait;
|
||||
use base64::Engine;
|
||||
use base64::engine::general_purpose::URL_SAFE_NO_PAD;
|
||||
use chrono::{DateTime, Utc};
|
||||
use fabro_http::HttpClient;
|
||||
use serde::{Deserialize, Serialize};
|
||||
use serde_json::json;
|
||||
|
|
@ -14,8 +15,8 @@ use crate::credential::{
|
|||
};
|
||||
use crate::strategy::AuthStrategy;
|
||||
|
||||
const DEVICE_AUTH_TIMEOUT: Duration = Duration::from_secs(15 * 60);
|
||||
const DEVICE_AUTH_POLL_INTERVAL: Duration = Duration::from_secs(2);
|
||||
const CODEX_DEVICE_VERIFICATION_URI: &str = "https://auth.openai.com/codex/device";
|
||||
|
||||
fn http_client() -> anyhow::Result<HttpClient> {
|
||||
#[cfg(test)]
|
||||
|
|
@ -80,14 +81,32 @@ pub fn extract_chatgpt_account_id(id_token: &str) -> Option<String> {
|
|||
})
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(untagged)]
|
||||
enum U64OrString {
|
||||
U64(u64),
|
||||
String(String),
|
||||
}
|
||||
|
||||
impl U64OrString {
|
||||
fn as_u64(&self) -> Option<u64> {
|
||||
match self {
|
||||
Self::U64(value) => Some(*value),
|
||||
Self::String(value) => value.parse().ok(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct DeviceCodeInitResponse {
|
||||
device_auth_id: String,
|
||||
user_code: String,
|
||||
#[serde(alias = "verificationUrl", alias = "verification_uri")]
|
||||
verification_uri: String,
|
||||
device_auth_id: String,
|
||||
user_code: String,
|
||||
#[serde(default)]
|
||||
expires_in: Option<u64>,
|
||||
interval: Option<U64OrString>,
|
||||
#[serde(default)]
|
||||
expires_in: Option<U64OrString>,
|
||||
#[serde(default)]
|
||||
expires_at: Option<DateTime<Utc>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
|
|
@ -96,21 +115,34 @@ struct DeviceCodePollResponse {
|
|||
status: Option<String>,
|
||||
#[serde(default)]
|
||||
authorization_code: Option<String>,
|
||||
#[serde(default)]
|
||||
code_verifier: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Serialize)]
|
||||
struct DeviceCodeInitRequest<'a> {
|
||||
client_id: &'a str,
|
||||
client_id: &'a str,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
code_challenge: Option<&'a str>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
scope: Option<String>,
|
||||
scope: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct PendingDeviceAuth {
|
||||
device_auth_id: String,
|
||||
user_code: String,
|
||||
poll_interval: Duration,
|
||||
deadline: Instant,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct DeviceAuthorization {
|
||||
authorization_code: String,
|
||||
code_verifier: String,
|
||||
}
|
||||
|
||||
pub struct CodexDeviceStrategy {
|
||||
config: OAuthConfig,
|
||||
device_auth_id: Option<String>,
|
||||
code_verifier: Option<String>,
|
||||
config: OAuthConfig,
|
||||
pending: Option<PendingDeviceAuth>,
|
||||
}
|
||||
|
||||
impl CodexDeviceStrategy {
|
||||
|
|
@ -118,27 +150,59 @@ impl CodexDeviceStrategy {
|
|||
pub fn new(config: OAuthConfig) -> Self {
|
||||
Self {
|
||||
config,
|
||||
device_auth_id: None,
|
||||
code_verifier: None,
|
||||
pending: None,
|
||||
}
|
||||
}
|
||||
|
||||
async fn poll_codex_device(&self, device_auth_id: &str) -> anyhow::Result<String> {
|
||||
fn init_expiry_seconds(payload: &DeviceCodeInitResponse) -> u64 {
|
||||
if let Some(expires_at) = payload.expires_at {
|
||||
let seconds = expires_at.signed_duration_since(Utc::now()).num_seconds();
|
||||
if seconds > 0 {
|
||||
return seconds.cast_unsigned();
|
||||
}
|
||||
}
|
||||
if let Some(seconds) = payload.expires_in.as_ref().and_then(U64OrString::as_u64) {
|
||||
return seconds;
|
||||
}
|
||||
300
|
||||
}
|
||||
|
||||
fn init_poll_interval(payload: &DeviceCodeInitResponse) -> Duration {
|
||||
Duration::from_secs(
|
||||
payload
|
||||
.interval
|
||||
.as_ref()
|
||||
.and_then(U64OrString::as_u64)
|
||||
.unwrap_or(DEVICE_AUTH_POLL_INTERVAL.as_secs()),
|
||||
)
|
||||
}
|
||||
|
||||
async fn poll_codex_device(
|
||||
&self,
|
||||
pending: &PendingDeviceAuth,
|
||||
) -> anyhow::Result<DeviceAuthorization> {
|
||||
let client = http_client()?;
|
||||
let deadline = Instant::now() + DEVICE_AUTH_TIMEOUT;
|
||||
let url = join_url(&self.config.auth_url, "/api/accounts/deviceauth/token");
|
||||
|
||||
loop {
|
||||
if Instant::now() >= deadline {
|
||||
if Instant::now() >= pending.deadline {
|
||||
return Err(anyhow::anyhow!("device auth timed out after 15 minutes"));
|
||||
}
|
||||
|
||||
let response = client
|
||||
.post(&url)
|
||||
.json(&json!({ "device_auth_id": device_auth_id }))
|
||||
.header("originator", "fabro")
|
||||
.json(&json!({
|
||||
"device_auth_id": pending.device_auth_id,
|
||||
"user_code": pending.user_code,
|
||||
}))
|
||||
.send()
|
||||
.await?;
|
||||
let status = response.status();
|
||||
if status.as_u16() == 403 || status.as_u16() == 404 {
|
||||
sleep(pending.poll_interval).await;
|
||||
continue;
|
||||
}
|
||||
if !status.is_success() {
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
return Err(anyhow::anyhow!(
|
||||
|
|
@ -147,17 +211,27 @@ impl CodexDeviceStrategy {
|
|||
}
|
||||
|
||||
let payload: DeviceCodePollResponse = response.json().await?;
|
||||
if let Some(code) = payload.authorization_code {
|
||||
return Ok(code);
|
||||
if let (Some(authorization_code), Some(code_verifier)) =
|
||||
(payload.authorization_code, payload.code_verifier)
|
||||
{
|
||||
return Ok(DeviceAuthorization {
|
||||
authorization_code,
|
||||
code_verifier,
|
||||
});
|
||||
}
|
||||
|
||||
match payload.status.as_deref() {
|
||||
Some("pending" | "running") | None => {
|
||||
sleep(DEVICE_AUTH_POLL_INTERVAL).await;
|
||||
Some("pending" | "running") => {
|
||||
sleep(pending.poll_interval).await;
|
||||
}
|
||||
Some(other) => {
|
||||
return Err(anyhow::anyhow!("device code exchange failed: {other}"));
|
||||
}
|
||||
None => {
|
||||
return Err(anyhow::anyhow!(
|
||||
"device auth response missing authorization_code or code_verifier"
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -166,18 +240,14 @@ impl CodexDeviceStrategy {
|
|||
#[async_trait]
|
||||
impl AuthStrategy for CodexDeviceStrategy {
|
||||
async fn init(&mut self) -> anyhow::Result<AuthContextRequest> {
|
||||
let pkce = self.config.use_pkce.then(fabro_oauth::generate_pkce);
|
||||
self.code_verifier = pkce.as_ref().map(|codes| codes.verifier.clone());
|
||||
|
||||
let client = http_client()?;
|
||||
let url = join_url(&self.config.auth_url, "/api/accounts/deviceauth/usercode");
|
||||
let response = client
|
||||
.post(&url)
|
||||
.header("originator", "fabro")
|
||||
.json(&DeviceCodeInitRequest {
|
||||
client_id: &self.config.client_id,
|
||||
code_challenge: pkce.as_ref().map(|codes| codes.challenge.as_str()),
|
||||
scope: (!self.config.scopes.is_empty())
|
||||
.then(|| self.config.scopes.join(" ")),
|
||||
client_id: &self.config.client_id,
|
||||
scope: (!self.config.scopes.is_empty()).then(|| self.config.scopes.join(" ")),
|
||||
})
|
||||
.send()
|
||||
.await?;
|
||||
|
|
@ -190,12 +260,19 @@ impl AuthStrategy for CodexDeviceStrategy {
|
|||
}
|
||||
|
||||
let payload: DeviceCodeInitResponse = response.json().await?;
|
||||
self.device_auth_id = Some(payload.device_auth_id);
|
||||
let expires_in = Self::init_expiry_seconds(&payload);
|
||||
let poll_interval = Self::init_poll_interval(&payload);
|
||||
self.pending = Some(PendingDeviceAuth {
|
||||
device_auth_id: payload.device_auth_id,
|
||||
user_code: payload.user_code.clone(),
|
||||
poll_interval,
|
||||
deadline: Instant::now() + Duration::from_secs(expires_in),
|
||||
});
|
||||
|
||||
Ok(AuthContextRequest::DeviceCode {
|
||||
user_code: payload.user_code,
|
||||
verification_uri: payload.verification_uri,
|
||||
expires_in: payload.expires_in.unwrap_or(900),
|
||||
user_code: payload.user_code,
|
||||
verification_uri: CODEX_DEVICE_VERIFICATION_URI.to_string(),
|
||||
expires_in,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -205,19 +282,19 @@ impl AuthStrategy for CodexDeviceStrategy {
|
|||
"expected device code confirmation response"
|
||||
)),
|
||||
AuthContextResponse::DeviceCodeConfirmed => {
|
||||
let device_auth_id = self
|
||||
.device_auth_id
|
||||
let pending = self
|
||||
.pending
|
||||
.take()
|
||||
.ok_or_else(|| anyhow::anyhow!("device auth flow was not initialized"))?;
|
||||
let authorization_code = self.poll_codex_device(&device_auth_id).await?;
|
||||
let authorization = self.poll_codex_device(&pending).await?;
|
||||
let token_response = fabro_oauth::exchange_code(
|
||||
fabro_oauth::OAuthEndpoint {
|
||||
token_url: &self.config.token_url,
|
||||
client_id: &self.config.client_id,
|
||||
},
|
||||
&authorization_code,
|
||||
&authorization.authorization_code,
|
||||
self.config.redirect_uri.as_deref(),
|
||||
self.code_verifier.as_deref(),
|
||||
Some(&authorization.code_verifier),
|
||||
)
|
||||
.await
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
|
|
@ -244,7 +321,27 @@ impl AuthStrategy for CodexDeviceStrategy {
|
|||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use httpmock::Method::POST;
|
||||
use httpmock::MockServer;
|
||||
|
||||
use super::*;
|
||||
use crate::strategy::codex_oauth_config;
|
||||
|
||||
fn test_config(server: &MockServer) -> OAuthConfig {
|
||||
OAuthConfig {
|
||||
auth_url: server.url(""),
|
||||
token_url: server.url("/oauth/token"),
|
||||
client_id: "test-client".to_string(),
|
||||
scopes: vec![
|
||||
"openid".to_string(),
|
||||
"profile".to_string(),
|
||||
"email".to_string(),
|
||||
"offline_access".to_string(),
|
||||
],
|
||||
redirect_uri: Some("https://auth.openai.com/deviceauth/callback".to_string()),
|
||||
use_pkce: false,
|
||||
}
|
||||
}
|
||||
|
||||
fn make_test_jwt(claims: &serde_json::Value) -> String {
|
||||
let header = URL_SAFE_NO_PAD.encode(r#"{"alg":"RS256"}"#);
|
||||
|
|
@ -280,4 +377,178 @@ mod tests {
|
|||
}));
|
||||
assert_eq!(extract_chatgpt_account_id(&jwt).as_deref(), Some("org_123"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn codex_oauth_config_disables_pkce() {
|
||||
let config = codex_oauth_config();
|
||||
assert!(!config.use_pkce);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn init_accepts_live_openai_shape_and_uses_hardcoded_verification_url() {
|
||||
let server = MockServer::start_async().await;
|
||||
let init_mock = server
|
||||
.mock_async(|when, then| {
|
||||
when.method(POST)
|
||||
.path("/api/accounts/deviceauth/usercode")
|
||||
.header("content-type", "application/json")
|
||||
.header("originator", "fabro")
|
||||
.json_body(serde_json::json!({
|
||||
"client_id": "test-client",
|
||||
"scope": "openid profile email offline_access"
|
||||
}));
|
||||
then.status(200)
|
||||
.header("content-type", "application/json")
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"device_auth_id": "deviceauth_123",
|
||||
"user_code": "ABCD-EFGH",
|
||||
"interval": "5",
|
||||
"expires_at": "2026-04-13T15:00:23.011951+00:00"
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
})
|
||||
.await;
|
||||
|
||||
let mut strategy = CodexDeviceStrategy::new(test_config(&server));
|
||||
|
||||
let request = strategy.init().await.unwrap();
|
||||
|
||||
assert_eq!(request, AuthContextRequest::DeviceCode {
|
||||
user_code: "ABCD-EFGH".to_string(),
|
||||
verification_uri: "https://auth.openai.com/codex/device".to_string(),
|
||||
expires_in: 300,
|
||||
});
|
||||
init_mock.assert_async().await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn complete_retries_403_and_uses_server_code_verifier() {
|
||||
let server = MockServer::start_async().await;
|
||||
let init_mock = server
|
||||
.mock_async(|when, then| {
|
||||
when.method(POST)
|
||||
.path("/api/accounts/deviceauth/usercode")
|
||||
.header("originator", "fabro")
|
||||
.header("content-type", "application/json")
|
||||
.json_body(serde_json::json!({
|
||||
"client_id": "test-client",
|
||||
"scope": "openid profile email offline_access"
|
||||
}));
|
||||
then.status(200)
|
||||
.header("content-type", "application/json")
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"device_auth_id": "deviceauth_123",
|
||||
"user_code": "ABCD-EFGH",
|
||||
"interval": "0",
|
||||
"expires_at": "2026-04-13T15:00:23.011951+00:00"
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
})
|
||||
.await;
|
||||
let pending_poll_mock = server
|
||||
.mock_async(|when, then| {
|
||||
when.method(POST)
|
||||
.path("/api/accounts/deviceauth/token")
|
||||
.header("content-type", "application/json")
|
||||
.header("originator", "fabro")
|
||||
.json_body(serde_json::json!({
|
||||
"device_auth_id": "deviceauth_123",
|
||||
"user_code": "ABCD-EFGH"
|
||||
}));
|
||||
then.status(403)
|
||||
.header("content-type", "application/json")
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"error": {
|
||||
"message": "authorization pending",
|
||||
"code": "authorization_pending"
|
||||
}
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
})
|
||||
.await;
|
||||
let success_poll_mock = server
|
||||
.mock_async(|when, then| {
|
||||
when.method(POST)
|
||||
.path("/api/accounts/deviceauth/token")
|
||||
.header("content-type", "application/json")
|
||||
.header("originator", "fabro")
|
||||
.json_body(serde_json::json!({
|
||||
"device_auth_id": "deviceauth_123",
|
||||
"user_code": "ABCD-EFGH"
|
||||
}));
|
||||
then.status(200)
|
||||
.header("content-type", "application/json")
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"authorization_code": "auth-code-123",
|
||||
"code_verifier": "returned-verifier-456"
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
})
|
||||
.await;
|
||||
let token_mock = server
|
||||
.mock_async(|when, then| {
|
||||
when.method(POST)
|
||||
.path("/oauth/token")
|
||||
.header("content-type", "application/x-www-form-urlencoded")
|
||||
.form_urlencoded_tuple("grant_type", "authorization_code")
|
||||
.form_urlencoded_tuple("code", "auth-code-123")
|
||||
.form_urlencoded_tuple(
|
||||
"redirect_uri",
|
||||
"https://auth.openai.com/deviceauth/callback",
|
||||
)
|
||||
.form_urlencoded_tuple("client_id", "test-client")
|
||||
.form_urlencoded_tuple("code_verifier", "returned-verifier-456");
|
||||
then.status(200)
|
||||
.header("content-type", "application/json")
|
||||
.body(
|
||||
serde_json::json!({
|
||||
"access_token": "new-access-token",
|
||||
"refresh_token": "new-refresh-token",
|
||||
"expires_in": 3600,
|
||||
"id_token": make_test_jwt(&json!({
|
||||
"chatgpt_account_id": "acct_123"
|
||||
}))
|
||||
})
|
||||
.to_string(),
|
||||
);
|
||||
})
|
||||
.await;
|
||||
|
||||
let mut strategy = CodexDeviceStrategy::new(test_config(&server));
|
||||
strategy.init().await.unwrap();
|
||||
|
||||
let complete = tokio::spawn(async move {
|
||||
strategy
|
||||
.complete(AuthContextResponse::DeviceCodeConfirmed)
|
||||
.await
|
||||
});
|
||||
while pending_poll_mock.calls_async().await == 0 {
|
||||
sleep(Duration::from_millis(10)).await;
|
||||
}
|
||||
assert!(pending_poll_mock.calls_async().await > 0);
|
||||
pending_poll_mock.delete_async().await;
|
||||
|
||||
let credential = complete.await.unwrap().unwrap();
|
||||
|
||||
let AuthDetails::CodexOAuth {
|
||||
tokens, account_id, ..
|
||||
} = credential.details
|
||||
else {
|
||||
panic!("expected codex oauth credential");
|
||||
};
|
||||
assert_eq!(tokens.access_token, "new-access-token");
|
||||
assert_eq!(tokens.refresh_token.as_deref(), Some("new-refresh-token"));
|
||||
assert_eq!(account_id.as_deref(), Some("acct_123"));
|
||||
init_mock.assert_async().await;
|
||||
success_poll_mock.assert_async().await;
|
||||
token_mock.assert_async().await;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@ pub fn codex_oauth_config() -> OAuthConfig {
|
|||
"offline_access".to_string(),
|
||||
],
|
||||
redirect_uri: Some(format!("{CODEX_AUTH_URL}/deviceauth/callback")),
|
||||
use_pkce: true,
|
||||
use_pkce: false,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -65,7 +65,7 @@ mod tests {
|
|||
assert_eq!(config.auth_url, CODEX_AUTH_URL);
|
||||
assert_eq!(config.token_url, CODEX_TOKEN_URL);
|
||||
assert_eq!(config.client_id, CODEX_CLIENT_ID);
|
||||
assert!(config.use_pkce);
|
||||
assert!(!config.use_pkce);
|
||||
assert!(config.scopes.contains(&"offline_access".to_string()));
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue