diff --git a/lib/crates/fabro-auth/src/credential.rs b/lib/crates/fabro-auth/src/credential.rs index 67d95d297..ac5479a77 100644 --- a/lib/crates/fabro-auth/src/credential.rs +++ b/lib/crates/fabro-auth/src/credential.rs @@ -42,6 +42,11 @@ pub struct OAuthTokens { pub expires_at: DateTime, } +pub(crate) fn expires_at_from_now(expires_in: Option) -> DateTime { + let seconds = i64::try_from(expires_in.unwrap_or(3600)).unwrap_or(i64::MAX); + Utc::now() + Duration::seconds(seconds) +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct OAuthConfig { pub auth_url: String, diff --git a/lib/crates/fabro-auth/src/refresh.rs b/lib/crates/fabro-auth/src/refresh.rs index a1d240d55..c1eb04be4 100644 --- a/lib/crates/fabro-auth/src/refresh.rs +++ b/lib/crates/fabro-auth/src/refresh.rs @@ -1,11 +1,4 @@ -use chrono::{Duration, Utc}; - -use crate::credential::{AuthCredential, AuthDetails, OAuthTokens}; - -fn expires_at_from_now(expires_in: Option) -> chrono::DateTime { - let seconds = i64::try_from(expires_in.unwrap_or(3600)).unwrap_or(i64::MAX); - Utc::now() + Duration::seconds(seconds) -} +use crate::credential::{AuthCredential, AuthDetails, OAuthTokens, expires_at_from_now}; pub async fn refresh_oauth_credential( credential: &AuthCredential, diff --git a/lib/crates/fabro-auth/src/resolve.rs b/lib/crates/fabro-auth/src/resolve.rs index a89178788..be9462fd3 100644 --- a/lib/crates/fabro-auth/src/resolve.rs +++ b/lib/crates/fabro-auth/src/resolve.rs @@ -5,6 +5,7 @@ use fabro_model::Provider; use fabro_vault::Vault; use shlex::try_quote; use tokio::sync::RwLock as AsyncRwLock; +use tokio::task::spawn_blocking; use crate::credential::{ApiKeyHeader, AuthCredential, AuthDetails, credential_id_for}; use crate::refresh::refresh_oauth_credential; @@ -107,7 +108,7 @@ impl CredentialResolver { })?; let refreshed_for_store = refreshed.clone(); let vault = Arc::clone(&self.vault); - tokio::task::spawn_blocking(move || { + spawn_blocking(move || { let mut vault = vault.blocking_write(); vault_set_credential(&mut vault, &credential_id, &refreshed_for_store) .map(|_| ()) @@ -130,7 +131,7 @@ impl CredentialResolver { self.to_api_credential(&vault, &credential), )), CredentialUsage::CliAgent(kind) => Ok(ResolvedCredential::Cli( - self.to_cli_credential(&credential, kind), + Self::to_cli_credential(&credential, kind), )), } } @@ -147,7 +148,7 @@ impl CredentialResolver { } } - for env_var in env_vars_for(provider) { + for env_var in provider.api_key_env_vars() { if let Some(value) = self.lookup_env_or_vault(vault, env_var) { return Ok(AuthCredential { provider, @@ -169,10 +170,13 @@ impl CredentialResolver { Provider::Anthropic => self.lookup_env_or_vault(vault, "ANTHROPIC_BASE_URL"), Provider::OpenAi => self.lookup_env_or_vault(vault, "OPENAI_BASE_URL"), Provider::Gemini => self.lookup_env_or_vault(vault, "GEMINI_BASE_URL"), - Provider::Kimi | Provider::Zai | Provider::Minimax | Provider::Inception => None, - Provider::OpenAiCompatible => None, + Provider::Kimi + | Provider::Zai + | Provider::Minimax + | Provider::Inception + | Provider::OpenAiCompatible => None, }; - let mut api_credential = match &credential.details { + match &credential.details { AuthDetails::ApiKey { key } => ApiCredential { provider: credential.provider, auth_header: match credential.provider { @@ -213,14 +217,10 @@ impl CredentialResolver { project_id: self.lookup_env_or_vault(vault, "OPENAI_PROJECT_ID"), } } - }; - if api_credential.provider == Provider::OpenAi && api_credential.codex_mode { - api_credential.base_url = Some("https://chatgpt.com/backend-api/codex".to_string()); } - api_credential } - fn to_cli_credential(&self, credential: &AuthCredential, kind: CliAgentKind) -> CliCredential { + fn to_cli_credential(credential: &AuthCredential, kind: CliAgentKind) -> CliCredential { let mut env_vars = HashMap::new(); let login_command = match (&credential.provider, &credential.details, kind) { (Provider::OpenAi, AuthDetails::ApiKey { key }, CliAgentKind::Codex) => { @@ -241,7 +241,7 @@ impl CredentialResolver { Some(codex_login_command(&tokens.access_token)) } (_, AuthDetails::ApiKey { key }, _) => { - if let Some(name) = env_vars_for(credential.provider).first() { + if let Some(name) = credential.provider.api_key_env_vars().first() { env_vars.insert((*name).to_string(), key.clone()); } None @@ -260,22 +260,17 @@ impl CredentialResolver { } fn codex_login_command(api_key: &str) -> String { - let quoted = try_quote(api_key) - .map(std::borrow::Cow::into_owned) - .unwrap_or_else(|_| api_key.to_string()); + let quoted = + try_quote(api_key).map_or_else(|_| api_key.to_string(), std::borrow::Cow::into_owned); format!("PATH=\"$HOME/.local/bin:$PATH\" echo {quoted} | codex login --with-api-key") } -fn env_vars_for(provider: Provider) -> &'static [&'static str] { - provider.api_key_env_vars() -} - fn credential_ids_for(provider: Provider, usage: CredentialUsage) -> &'static [&'static str] { match (provider, usage) { - (Provider::OpenAi, CredentialUsage::ApiRequest) => &["openai"], (Provider::OpenAi, CredentialUsage::CliAgent(CliAgentKind::Codex)) => { &["openai_codex", "openai"] } + (Provider::OpenAi, _) => &["openai"], (Provider::Anthropic, _) => &["anthropic"], (Provider::Gemini, _) => &["gemini"], (Provider::Kimi, _) => &["kimi"], @@ -283,7 +278,6 @@ fn credential_ids_for(provider: Provider, usage: CredentialUsage) -> &'static [& (Provider::Minimax, _) => &["minimax"], (Provider::Inception, _) => &["inception"], (Provider::OpenAiCompatible, _) => &[], - (Provider::OpenAi, _) => &["openai"], } } diff --git a/lib/crates/fabro-auth/src/strategies/codex_device.rs b/lib/crates/fabro-auth/src/strategies/codex_device.rs index 60bc22493..d5d5442ec 100644 --- a/lib/crates/fabro-auth/src/strategies/codex_device.rs +++ b/lib/crates/fabro-auth/src/strategies/codex_device.rs @@ -9,7 +9,9 @@ use serde_json::json; use tokio::time::sleep; use crate::context::{AuthContextRequest, AuthContextResponse}; -use crate::credential::{AuthCredential, AuthDetails, OAuthConfig, OAuthTokens}; +use crate::credential::{ + AuthCredential, AuthDetails, OAuthConfig, OAuthTokens, expires_at_from_now, +}; use crate::strategy::AuthStrategy; const DEVICE_AUTH_TIMEOUT: Duration = Duration::from_secs(15 * 60); @@ -30,11 +32,6 @@ fn join_url(base: &str, path: &str) -> String { format!("{}{}", base.trim_end_matches('/'), path) } -fn expires_at_from_now(expires_in: Option) -> chrono::DateTime { - let seconds = i64::try_from(expires_in.unwrap_or(3600)).unwrap_or(i64::MAX); - chrono::Utc::now() + chrono::Duration::seconds(seconds) -} - #[derive(Debug, Deserialize)] struct JwtPayload { #[serde(default)] @@ -155,7 +152,7 @@ impl CodexDeviceStrategy { } match payload.status.as_deref() { - Some("pending") | Some("running") | None => { + Some("pending" | "running") | None => { sleep(DEVICE_AUTH_POLL_INTERVAL).await; } Some(other) => { diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 58d965e24..6f6907617 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -711,7 +711,7 @@ async fn persist_vault_secrets( .body(CreateSecretRequest { name: secret.name.clone(), value: secret.value.clone(), - type_: secret.type_.clone(), + type_: secret.type_, description: secret.description.clone(), }) .send() @@ -726,14 +726,14 @@ async fn persist_vault_secrets( store.set( &secret.name, &secret.value, - local_secret_type(&secret.type_), + local_secret_type(secret.type_), secret.description.as_deref(), )?; } Ok(()) } -fn local_secret_type(secret_type: &ApiSecretType) -> SecretType { +fn local_secret_type(secret_type: ApiSecretType) -> SecretType { match secret_type { ApiSecretType::Environment => SecretType::Environment, ApiSecretType::File => SecretType::File, diff --git a/lib/crates/fabro-cli/src/shared/provider_auth.rs b/lib/crates/fabro-cli/src/shared/provider_auth.rs index d8fc83411..e9d832743 100644 --- a/lib/crates/fabro-cli/src/shared/provider_auth.rs +++ b/lib/crates/fabro-cli/src/shared/provider_auth.rs @@ -36,16 +36,7 @@ pub(crate) fn provider_key_url(provider: Provider) -> &'static str { } pub(crate) fn provider_display_name(provider: Provider) -> &'static str { - match provider { - Provider::Anthropic => "Anthropic", - Provider::OpenAi => "OpenAI", - Provider::Gemini => "Gemini", - Provider::Kimi => "Kimi", - Provider::Zai => "Zai", - Provider::Minimax => "Minimax", - Provider::Inception => "Inception", - Provider::OpenAiCompatible => "OpenAI Compatible", - } + provider.display_name() } // --------------------------------------------------------------------------- @@ -176,25 +167,18 @@ pub(crate) async fn authenticate_provider_with_method( ) -> Result { let mut strategy = strategy_for(provider, method); let request = strategy.init().await?; - present_to_user(&request, s, printer)?; + present_to_user(&request, s, printer); let response = await_user_response(&request, s, printer).await?; strategy.complete(response).await } -pub(crate) fn present_to_user( - request: &AuthContextRequest, - s: &Styles, - printer: Printer, -) -> Result<()> { +pub(crate) fn present_to_user(request: &AuthContextRequest, s: &Styles, printer: Printer) { match request { AuthContextRequest::ApiKey { provider, env_var_names, } => { - let env_var = env_var_names - .first() - .map(String::as_str) - .unwrap_or("API_KEY"); + let env_var = env_var_names.first().map_or("API_KEY", String::as_str); let url = provider_key_url(*provider); fabro_util::printerr!( printer, @@ -224,7 +208,6 @@ pub(crate) fn present_to_user( ); } } - Ok(()) } pub(crate) async fn await_user_response( diff --git a/lib/crates/fabro-llm/src/client.rs b/lib/crates/fabro-llm/src/client.rs index 3b2db727b..f589c47be 100644 --- a/lib/crates/fabro-llm/src/client.rs +++ b/lib/crates/fabro-llm/src/client.rs @@ -412,8 +412,7 @@ impl Client { fn auth_value(auth_header: &ApiKeyHeader) -> String { match auth_header { - ApiKeyHeader::Bearer(value) => value.clone(), - ApiKeyHeader::Custom { value, .. } => value.clone(), + ApiKeyHeader::Bearer(value) | ApiKeyHeader::Custom { value, .. } => value.clone(), } } diff --git a/lib/crates/fabro-model/src/provider.rs b/lib/crates/fabro-model/src/provider.rs index e5666850a..be192efc8 100644 --- a/lib/crates/fabro-model/src/provider.rs +++ b/lib/crates/fabro-model/src/provider.rs @@ -81,6 +81,21 @@ impl Provider { .unwrap_or(Self::Anthropic) } + /// Human-readable display name for the provider. + #[must_use] + pub fn display_name(self) -> &'static str { + match self { + Self::Anthropic => "Anthropic", + Self::OpenAi => "OpenAI", + Self::Gemini => "Gemini", + Self::Kimi => "Kimi", + Self::Zai => "Zai", + Self::Minimax => "Minimax", + Self::Inception => "Inception", + Self::OpenAiCompatible => "OpenAI Compatible", + } + } + /// Stable lowercase string representation used in `Request.provider`, /// adapter names, and other serialization boundaries. #[must_use] diff --git a/lib/crates/fabro-server/src/server_secrets.rs b/lib/crates/fabro-server/src/server_secrets.rs index aa9b9ff5b..232f96d92 100644 --- a/lib/crates/fabro-server/src/server_secrets.rs +++ b/lib/crates/fabro-server/src/server_secrets.rs @@ -109,8 +109,7 @@ impl ProviderCredentials { .await { Ok(ResolvedCredential::Api(credential)) => api_credentials.push(credential), - Ok(ResolvedCredential::Cli(_)) => {} - Err(ResolveError::NotConfigured(_)) => {} + Ok(ResolvedCredential::Cli(_)) | Err(ResolveError::NotConfigured(_)) => {} Err(err) => auth_issues.push((*provider, err)), } } @@ -131,32 +130,19 @@ pub(crate) struct LlmClientResult { pub auth_issues: Vec<(Provider, ResolveError)>, } -pub(crate) fn provider_display_name(provider: Provider) -> &'static str { - match provider { - Provider::Anthropic => "Anthropic", - Provider::OpenAi => "OpenAI", - Provider::Gemini => "Gemini", - Provider::Kimi => "Kimi", - Provider::Zai => "Zai", - Provider::Minimax => "Minimax", - Provider::Inception => "Inception", - Provider::OpenAiCompatible => "OpenAI Compatible", - } -} - pub(crate) fn auth_issue_message(provider: Provider, err: &ResolveError) -> String { match err { ResolveError::NotConfigured(_) => { - format!("{} is not configured", provider_display_name(provider)) + format!("{} is not configured", provider.display_name()) } ResolveError::RefreshFailed { source, .. } => format!( "{} requires re-authentication: {}", - provider_display_name(provider), + provider.display_name(), source ), ResolveError::RefreshTokenMissing(_) => format!( "{} requires re-authentication: refresh token missing", - provider_display_name(provider) + provider.display_name() ), } }