From 0337e5ee96a0aed1180d82b793a04f2f52fe2f47 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Tue, 31 Mar 2026 13:23:43 -0400 Subject: [PATCH] Rename fabro-openai-oauth to fabro-oauth --- Cargo.lock | 4 +- lib/crates/fabro-cli/Cargo.toml | 2 +- lib/crates/fabro-cli/src/shared/mod.rs | 1 + lib/crates/fabro-cli/src/shared/openai_jwt.rs | 138 +++++ .../fabro-cli/src/shared/provider_auth.rs | 26 +- .../Cargo.toml | 4 +- lib/crates/fabro-oauth/examples/login.rs | 32 + .../src/lib.rs | 573 +++++------------- .../fabro-openai-oauth/examples/login.rs | 24 - 9 files changed, 347 insertions(+), 457 deletions(-) create mode 100644 lib/crates/fabro-cli/src/shared/openai_jwt.rs rename lib/crates/{fabro-openai-oauth => fabro-oauth}/Cargo.toml (84%) create mode 100644 lib/crates/fabro-oauth/examples/login.rs rename lib/crates/{fabro-openai-oauth => fabro-oauth}/src/lib.rs (63%) delete mode 100644 lib/crates/fabro-openai-oauth/examples/login.rs diff --git a/Cargo.lock b/Cargo.lock index e44583067..70450b0a7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1492,7 +1492,7 @@ dependencies = [ "fabro-macros", "fabro-mcp", "fabro-model", - "fabro-openai-oauth", + "fabro-oauth", "fabro-proctitle", "fabro-retro", "fabro-sandbox", @@ -1726,7 +1726,7 @@ dependencies = [ ] [[package]] -name = "fabro-openai-oauth" +name = "fabro-oauth" version = "0.176.2" dependencies = [ "axum", diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 7f3d138f3..390d47e5d 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -22,7 +22,7 @@ workspace = true fabro-config = { path = "../fabro-config" } fabro-llm = { path = "../fabro-llm" } fabro-model = { path = "../fabro-model" } -fabro-openai-oauth = { path = "../fabro-openai-oauth" } +fabro-oauth = { path = "../fabro-oauth" } fabro-github = { path = "../fabro-github" } fabro-agent = { path = "../fabro-agent" } fabro-devcontainer = { path = "../fabro-devcontainer" } diff --git a/lib/crates/fabro-cli/src/shared/mod.rs b/lib/crates/fabro-cli/src/shared/mod.rs index 5166958a4..4aca2e627 100644 --- a/lib/crates/fabro-cli/src/shared/mod.rs +++ b/lib/crates/fabro-cli/src/shared/mod.rs @@ -1,4 +1,5 @@ pub(crate) mod github; +pub(crate) mod openai_jwt; pub(crate) mod provider_auth; mod utilities; diff --git a/lib/crates/fabro-cli/src/shared/openai_jwt.rs b/lib/crates/fabro-cli/src/shared/openai_jwt.rs new file mode 100644 index 000000000..712fbf7fd --- /dev/null +++ b/lib/crates/fabro-cli/src/shared/openai_jwt.rs @@ -0,0 +1,138 @@ +use base64::Engine; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use serde::Deserialize; + +pub(crate) const DEFAULT_CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; +pub(crate) const DEFAULT_ISSUER: &str = "https://auth.openai.com"; +pub(crate) const OAUTH_PORT: u16 = 1455; + +#[derive(Deserialize)] +struct JwtPayload { + #[serde(default)] + chatgpt_account_id: Option, + #[serde(default, rename = "https://api.openai.com/auth")] + auth_claim: Option, + #[serde(default)] + organizations: Option>, +} + +#[derive(Deserialize)] +struct AuthClaim { + #[serde(default)] + chatgpt_account_id: Option, +} + +#[derive(Deserialize)] +struct Organization { + #[serde(default)] + id: Option, +} + +fn parse_jwt_payload(token: &str) -> Option { + let parts: Vec<&str> = token.split('.').collect(); + if parts.len() != 3 { + return None; + } + let payload_bytes = URL_SAFE_NO_PAD.decode(parts[1]).ok()?; + serde_json::from_slice(&payload_bytes).ok() +} + +pub(crate) fn extract_account_id(id_token: &str) -> Option { + let payload = parse_jwt_payload(id_token)?; + payload + .chatgpt_account_id + .or_else(|| { + payload + .auth_claim + .and_then(|claim| claim.chatgpt_account_id) + }) + .or_else(|| { + payload + .organizations + .and_then(|orgs| orgs.into_iter().next()) + .and_then(|org| org.id) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn make_test_jwt(claims: &serde_json::Value) -> String { + let header = URL_SAFE_NO_PAD.encode(r#"{"alg":"RS256"}"#); + let payload = URL_SAFE_NO_PAD.encode(serde_json::to_string(claims).unwrap()); + format!("{header}.{payload}.signature") + } + + #[test] + fn parse_jwt_with_chatgpt_account_id() { + let jwt = make_test_jwt(&serde_json::json!({ + "chatgpt_account_id": "acct_123" + })); + let payload = parse_jwt_payload(&jwt).unwrap(); + assert_eq!(payload.chatgpt_account_id.as_deref(), Some("acct_123")); + } + + #[test] + fn parse_jwt_with_nested_auth_claim() { + let jwt = make_test_jwt(&serde_json::json!({ + "https://api.openai.com/auth": { + "chatgpt_account_id": "acct_nested" + } + })); + let payload = parse_jwt_payload(&jwt).unwrap(); + assert_eq!( + payload + .auth_claim + .and_then(|claim| claim.chatgpt_account_id) + .as_deref(), + Some("acct_nested") + ); + } + + #[test] + fn parse_jwt_invalid_format() { + assert!(parse_jwt_payload("not-a-jwt").is_none()); + } + + #[test] + fn parse_jwt_invalid_base64() { + assert!(parse_jwt_payload("header.!!!invalid!!!.sig").is_none()); + } + + #[test] + fn extract_account_id_prefers_top_level() { + let jwt = make_test_jwt(&serde_json::json!({ + "chatgpt_account_id": "top_level", + "https://api.openai.com/auth": { + "chatgpt_account_id": "nested" + }, + "organizations": [{"id": "org"}] + })); + assert_eq!(extract_account_id(&jwt).as_deref(), Some("top_level")); + } + + #[test] + fn extract_account_id_falls_back_to_nested() { + let jwt = make_test_jwt(&serde_json::json!({ + "https://api.openai.com/auth": { + "chatgpt_account_id": "nested" + } + })); + assert_eq!(extract_account_id(&jwt).as_deref(), Some("nested")); + } + + #[test] + fn extract_account_id_falls_back_to_first_organization() { + let jwt = make_test_jwt(&serde_json::json!({ + "organizations": [{"id": "org_456"}] + })); + assert_eq!(extract_account_id(&jwt).as_deref(), Some("org_456")); + } + + #[test] + fn extract_account_id_none_when_missing() { + let jwt = make_test_jwt(&serde_json::json!({})); + assert!(extract_account_id(&jwt).is_none()); + } +} diff --git a/lib/crates/fabro-cli/src/shared/provider_auth.rs b/lib/crates/fabro-cli/src/shared/provider_auth.rs index 6088371fd..738734534 100644 --- a/lib/crates/fabro-cli/src/shared/provider_auth.rs +++ b/lib/crates/fabro-cli/src/shared/provider_auth.rs @@ -12,6 +12,7 @@ use fabro_util::terminal::Styles; use tokio::task::spawn_blocking; use tokio::time::timeout; +use super::openai_jwt; use crate::commands::doctor; // --------------------------------------------------------------------------- @@ -80,20 +81,27 @@ pub(crate) async fn run_openai_oauth_or_api_key(s: &Styles) -> Result { tracing::info!("OpenAI OAuth browser flow completed"); - let account_id = fabro_openai_oauth::extract_account_id(&tokens); - let pairs = openai_oauth_env_pairs( - &tokens.access_token, - &tokens.refresh_token, - account_id.as_deref(), - ); + let account_id = tokens + .id_token + .as_deref() + .and_then(openai_jwt::extract_account_id); + let refresh_token = tokens + .refresh_token + .as_deref() + .ok_or_else(|| anyhow::anyhow!("OpenAI did not return a refresh token"))?; + let pairs = + openai_oauth_env_pairs(&tokens.access_token, refresh_token, account_id.as_deref()); eprintln!( " {} OpenAI configured via browser login", s.green.apply_to("✔") diff --git a/lib/crates/fabro-openai-oauth/Cargo.toml b/lib/crates/fabro-oauth/Cargo.toml similarity index 84% rename from lib/crates/fabro-openai-oauth/Cargo.toml rename to lib/crates/fabro-oauth/Cargo.toml index 630c637f2..150fa898a 100644 --- a/lib/crates/fabro-openai-oauth/Cargo.toml +++ b/lib/crates/fabro-oauth/Cargo.toml @@ -1,10 +1,10 @@ [package] -name = "fabro-openai-oauth" +name = "fabro-oauth" edition.workspace = true version.workspace = true publish = false license.workspace = true -description = "OpenAI OAuth PKCE token acquisition for Fabro" +description = "Generic OAuth 2.0 PKCE token acquisition for Fabro" [lib] doctest = false diff --git a/lib/crates/fabro-oauth/examples/login.rs b/lib/crates/fabro-oauth/examples/login.rs new file mode 100644 index 000000000..e8f523112 --- /dev/null +++ b/lib/crates/fabro-oauth/examples/login.rs @@ -0,0 +1,32 @@ +use std::env; + +use fabro_oauth::run_browser_flow; + +#[tokio::main] +async fn main() { + let issuer = env::var("OAUTH_ISSUER").expect("set OAUTH_ISSUER"); + let client_id = env::var("OAUTH_CLIENT_ID").expect("set OAUTH_CLIENT_ID"); + let scope = env::var("OAUTH_SCOPE").unwrap_or_else(|_| "openid profile email".to_string()); + let port: u16 = env::var("OAUTH_PORT") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(0); + let callback_path = env::var("OAUTH_CALLBACK_PATH").unwrap_or_else(|_| "/callback".to_string()); + + match run_browser_flow(&issuer, &client_id, &scope, port, &callback_path).await { + Ok(tokens) => { + println!("Login successful!"); + println!( + "Access token: {}...", + &tokens.access_token[..20.min(tokens.access_token.len())] + ); + if let Some(expires_in) = tokens.expires_in { + println!("Expires in: {expires_in}s"); + } + } + Err(e) => { + eprintln!("Login failed: {e}"); + std::process::exit(1); + } + } +} diff --git a/lib/crates/fabro-openai-oauth/src/lib.rs b/lib/crates/fabro-oauth/src/lib.rs similarity index 63% rename from lib/crates/fabro-openai-oauth/src/lib.rs rename to lib/crates/fabro-oauth/src/lib.rs index a6227673f..705bc8a8a 100644 --- a/lib/crates/fabro-openai-oauth/src/lib.rs +++ b/lib/crates/fabro-oauth/src/lib.rs @@ -10,11 +10,6 @@ use serde::Deserialize; use sha2::{Digest, Sha256}; use tokio::net::TcpListener; use tokio::sync::oneshot; -use tokio::time; - -pub const DEFAULT_CLIENT_ID: &str = "app_EMoamEEZ73f0CkXaXp7hrann"; -pub const DEFAULT_ISSUER: &str = "https://auth.openai.com"; -pub const OAUTH_PORT: u16 = 1455; // --------------------------------------------------------------------------- // PKCE @@ -79,6 +74,7 @@ pub fn build_authorize_url( issuer: &str, client_id: &str, redirect_uri: &str, + scope: &str, pkce: &PkceCodes, state: &str, ) -> String { @@ -86,7 +82,7 @@ pub fn build_authorize_url( ("response_type", "code"), ("client_id", client_id), ("redirect_uri", redirect_uri), - ("scope", "openid profile email offline_access"), + ("scope", scope), ("code_challenge", &pkce.challenge), ("code_challenge_method", "S256"), ("state", state), @@ -100,72 +96,12 @@ pub fn build_authorize_url( #[derive(Debug, Deserialize)] pub struct TokenResponse { - pub id_token: String, + pub id_token: Option, pub access_token: String, - pub refresh_token: String, + pub refresh_token: Option, pub expires_in: Option, } -// --------------------------------------------------------------------------- -// JWT claims -// --------------------------------------------------------------------------- - -pub struct IdTokenClaims { - pub chatgpt_account_id: Option, -} - -#[derive(Deserialize)] -struct JwtPayload { - #[serde(default)] - chatgpt_account_id: Option, - #[serde(default, rename = "https://api.openai.com/auth")] - auth_claim: Option, - #[serde(default)] - organizations: Option>, -} - -#[derive(Deserialize)] -struct AuthClaim { - #[serde(default)] - chatgpt_account_id: Option, -} - -#[derive(Deserialize)] -struct Organization { - #[serde(default)] - id: Option, -} - -fn parse_jwt_payload(token: &str) -> Option { - let parts: Vec<&str> = token.split('.').collect(); - if parts.len() != 3 { - return None; - } - let payload_bytes = URL_SAFE_NO_PAD.decode(parts[1]).ok()?; - serde_json::from_slice(&payload_bytes).ok() -} - -pub fn parse_jwt_claims(token: &str) -> Option { - let payload = parse_jwt_payload(token)?; - let chatgpt_account_id = payload - .chatgpt_account_id - .or_else(|| payload.auth_claim.and_then(|a| a.chatgpt_account_id)); - Some(IdTokenClaims { chatgpt_account_id }) -} - -pub fn extract_account_id(tokens: &TokenResponse) -> Option { - let payload = parse_jwt_payload(&tokens.id_token)?; - payload - .chatgpt_account_id - .or_else(|| payload.auth_claim.and_then(|a| a.chatgpt_account_id)) - .or_else(|| { - payload - .organizations - .and_then(|orgs| orgs.into_iter().next()) - .and_then(|org| org.id) - }) -} - // --------------------------------------------------------------------------- // Token exchange // --------------------------------------------------------------------------- @@ -256,107 +192,6 @@ pub async fn refresh_access_token( Ok(tokens) } -// --------------------------------------------------------------------------- -// Device flow -// --------------------------------------------------------------------------- - -#[derive(Debug, Deserialize)] -pub struct DeviceAuthResponse { - pub device_auth_id: String, - pub user_code: String, - pub interval: u64, -} - -pub async fn initiate_device_flow( - client: &reqwest::Client, - issuer: &str, - client_id: &str, -) -> Result { - let url = format!("{issuer}/api/accounts/deviceauth/usercode"); - let resp = client - .post(&url) - .json(&serde_json::json!({ "client_id": client_id })) - .send() - .await - .map_err(|e| format!("Device flow initiation failed: {e}"))?; - - let status = resp.status(); - if !status.is_success() { - let body_text = resp.text().await.unwrap_or_default(); - return Err(format!( - "Device flow initiation failed ({status}): {body_text}" - )); - } - - let device: DeviceAuthResponse = resp - .json() - .await - .map_err(|e| format!("Failed to parse device flow response: {e}"))?; - - tracing::info!("Device flow initiated"); - Ok(device) -} - -#[derive(Deserialize)] -struct DevicePollResponse { - #[serde(default)] - code: Option, - #[serde(default)] - error: Option, -} - -pub async fn poll_device_flow( - client: &reqwest::Client, - issuer: &str, - client_id: &str, - device: &DeviceAuthResponse, -) -> Result { - let poll_url = format!("{issuer}/api/accounts/deviceauth/token"); - let redirect_uri = format!("http://localhost:{OAUTH_PORT}/auth/callback"); - let mut attempt = 0u32; - - loop { - attempt += 1; - let resp = client - .post(&poll_url) - .json(&serde_json::json!({ - "client_id": client_id, - "device_auth_id": device.device_auth_id, - })) - .send() - .await - .map_err(|e| format!("Device flow poll failed: {e}"))?; - - let poll: DevicePollResponse = resp - .json() - .await - .map_err(|e| format!("Failed to parse device poll response: {e}"))?; - - if let Some(code) = poll.code { - tracing::info!("Device flow completed"); - return exchange_code_for_tokens(client, issuer, client_id, &code, &redirect_uri, "") - .await; - } - - if let Some(ref error) = poll.error { - if error == "authorization_pending" { - tracing::debug!(attempt, "Device flow authorization pending"); - if device.interval > 0 { - time::sleep(std::time::Duration::from_secs(device.interval)).await; - } - continue; - } - if error == "expired_token" { - tracing::error!("Device flow expired"); - return Err("Device flow authorization expired".to_string()); - } - return Err(format!("Device flow error: {error}")); - } - - return Err("Unexpected device poll response".to_string()); - } -} - // --------------------------------------------------------------------------- // Callback server // --------------------------------------------------------------------------- @@ -369,10 +204,36 @@ struct CallbackParams { error_description: Option, } +fn validate_callback_path(path: &str) -> Result<(), String> { + if path.is_empty() { + return Err("Callback path must not be empty".to_string()); + } + if !path.starts_with('/') { + return Err(format!("Callback path must start with '/': {path}")); + } + if path + .split('/') + .skip(1) + .any(|segment| segment.starts_with(':') || segment.starts_with('*')) + { + return Err(format!( + "Callback path must not contain route parameters: {path}" + )); + } + Ok(()) +} + +fn build_redirect_uri(port: u16, path: &str) -> String { + format!("http://localhost:{port}{path}") +} + pub async fn start_callback_server( port: u16, + path: &str, expected_state: String, ) -> Result<(u16, oneshot::Receiver>), String> { + validate_callback_path(path)?; + let listener = TcpListener::bind(format!("localhost:{port}")) .await .map_err(|e| format!("Failed to bind callback server: {e}"))?; @@ -387,9 +248,10 @@ pub async fn start_callback_server( let code_tx = std::sync::Arc::new(std::sync::Mutex::new(Some(code_tx))); let shutdown_tx = std::sync::Arc::new(std::sync::Mutex::new(Some(shutdown_tx))); let expected_state = std::sync::Arc::new(expected_state); + let callback_path = path.to_string(); let app = axum::Router::new().route( - "/auth/callback", + callback_path.as_str(), get( move |Query(params): Query| async move { if params.state != *expected_state { @@ -463,7 +325,7 @@ pub async fn start_callback_server( -Arc +Authorization