diff --git a/lib/crates/fabro-cli/src/commands/auth/login.rs b/lib/crates/fabro-cli/src/commands/auth/login.rs index 227ff8936..c7c85f42d 100644 --- a/lib/crates/fabro-cli/src/commands/auth/login.rs +++ b/lib/crates/fabro-cli/src/commands/auth/login.rs @@ -3,7 +3,7 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use chrono::{DateTime, Utc}; use fabro_api::types; -use fabro_client::{AuthEntry, AuthStore, StoredSubject}; +use fabro_client::{AuthEntry, AuthStore, StoredSubject, ensure_refresh_target_transport}; use fabro_http::header::CONTENT_TYPE; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::CliLayer; @@ -96,14 +96,7 @@ pub(super) async fn login_command( } }; - match target.loopback_classification()? { - fabro_client::LoopbackClassification::Https - | fabro_client::LoopbackClassification::LoopbackHttp - | fabro_client::LoopbackClassification::UnixSocket => {} - fabro_client::LoopbackClassification::Rejected => { - bail!("{}", token_transport_error(&target)); - } - } + ensure_refresh_target_transport(&target)?; let tokens = exchange_cli_token(&target, &code, &pkce.verifier, &redirect_uri).await?; let entry = AuthEntry { @@ -129,7 +122,7 @@ pub(super) async fn login_command( #[cfg(unix)] async fn fetch_cli_auth_config(target: &ServerTarget) -> Result { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let client = fabro_api::ApiClient::new_with_client(&base_url, http_client); client .get_cli_auth_config() @@ -146,7 +139,7 @@ async fn exchange_cli_token( code_verifier: &str, redirect_uri: &str, ) -> Result { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let response = http_client .post(format!("{base_url}/auth/cli/token")) .header(CONTENT_TYPE, "application/json") @@ -241,12 +234,6 @@ fn login_failure_message(error_code: &str, error_description: Option<&str>) -> S } } -fn token_transport_error(target: &ServerTarget) -> String { - format!( - "Refusing to send refresh-token credentials over plaintext HTTP to a non-loopback host ({target}). Use HTTPS, or bind the server to 127.0.0.1 / ::1." - ) -} - fn identity_summary(subject: &StoredSubject) -> String { if !subject.name.is_empty() && !subject.email.is_empty() { format!("{} ({} <{}>)", subject.login, subject.name, subject.email) diff --git a/lib/crates/fabro-cli/src/commands/auth/logout.rs b/lib/crates/fabro-cli/src/commands/auth/logout.rs index 4743eb42b..4b72ef5c1 100644 --- a/lib/crates/fabro-cli/src/commands/auth/logout.rs +++ b/lib/crates/fabro-cli/src/commands/auth/logout.rs @@ -58,7 +58,7 @@ pub(super) async fn logout_command( } async fn revoke_remote_session(target: &ServerTarget, entry: &AuthEntry) -> Result<()> { - let (http_client, base_url) = user_config::build_public_http_client(target)?; + let (http_client, base_url) = target.build_public_http_client()?; let response = http_client .post(format!("{base_url}/auth/cli/logout")) .header(AUTHORIZATION, format!("Bearer {}", entry.refresh_token)) diff --git a/lib/crates/fabro-cli/src/commands/run/create.rs b/lib/crates/fabro-cli/src/commands/run/create.rs index 6ee251680..e1c93496c 100644 --- a/lib/crates/fabro-cli/src/commands/run/create.rs +++ b/lib/crates/fabro-cli/src/commands/run/create.rs @@ -13,7 +13,7 @@ use super::overrides::run_args_layer; use crate::args::RunArgs; use crate::command_context::CommandContext; use crate::manifest_builder::{ManifestBuildInput, build_run_manifest, run_manifest_args}; -use crate::user_config::{self, ServerTarget}; +use crate::user_config; pub(crate) struct CreatedRun { pub(crate) run_id: RunId, diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 4f12f2c0c..337a34530 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -5,10 +5,10 @@ use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use fabro_client::{ AuthStore, Credential, CredentialFallback, OAuthSession, ServerTarget, TransportConnector, + apply_bearer_token_auth, }; pub(crate) use fabro_client::{Client, RunEventStream}; use fabro_config::Storage; -use fabro_http::header::AUTHORIZATION; use fabro_server::bind::Bind; pub(crate) use fabro_types::RunProjection; use fabro_types::settings::SettingsLayer; @@ -264,19 +264,6 @@ async fn wait_for_local_dev_token(storage_dir: &Path) -> Result { ); } -fn apply_bearer_token_auth( - builder: fabro_http::HttpClientBuilder, - token: &str, -) -> Result { - let mut headers = fabro_http::HeaderMap::new(); - headers.insert( - AUTHORIZATION, - fabro_http::HeaderValue::from_str(&format!("Bearer {token}")) - .context("invalid dev token header value")?, - ); - Ok(builder.default_headers(headers)) -} - async fn build_authed_unix_socket_http_client( path: &Path, storage_dir: Option<&Path>, diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index c151caa4e..3fa3710da 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -61,12 +61,6 @@ pub(crate) fn apply_storage_dir_override( layer } -pub(crate) fn build_public_http_client( - target: &ServerTarget, -) -> Result<(fabro_http::HttpClient, String)> { - target.build_public_http_client() -} - /// Pull the resolved CLI target configuration out of `[cli.target]`. /// Returns either an http(s) URL or a unix socket path. fn cli_target_from_settings(settings: &CliSettings) -> Option { diff --git a/lib/crates/fabro-client/src/auth_store.rs b/lib/crates/fabro-client/src/auth_store.rs index 01185ac72..e436e3e78 100644 --- a/lib/crates/fabro-client/src/auth_store.rs +++ b/lib/crates/fabro-client/src/auth_store.rs @@ -120,10 +120,6 @@ impl AuthStore { } pub fn get(&self, target: &ServerTarget) -> Result, AuthStoreError> { - if !self.path.exists() { - return Ok(None); - } - let key = key_for_target(target); self.with_shared_lock(|| { let file = self.read_auth_file()?; @@ -143,7 +139,7 @@ impl AuthStore { let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { - let mut file = self.read_auth_file_if_exists()?; + let mut file = self.read_auth_file()?; file.servers.insert(key, entry); self.write_auth_file(&file) }) @@ -159,26 +155,20 @@ impl AuthStore { #[cfg(unix)] { - if !self.path.exists() { - return Ok(false); - } - let key = key_for_target(target); self.ensure_parent_dir()?; self.with_exclusive_lock(|| { - let mut file = self.read_auth_file_if_exists()?; + let mut file = self.read_auth_file()?; let removed = file.servers.remove(&key).is_some(); - self.write_auth_file(&file)?; + if removed { + self.write_auth_file(&file)?; + } Ok(removed) }) } } pub fn list(&self) -> Result, AuthStoreError> { - if !self.path.exists() { - return Ok(Vec::new()); - } - self.with_shared_lock(|| { let file = self.read_auth_file()?; file.servers @@ -189,21 +179,19 @@ impl AuthStore { } fn read_auth_file(&self) -> Result { - let contents = fs::read_to_string(&self.path).map_err(|source| AuthStoreError::Read { - path: self.path.clone(), - source, - })?; - serde_json::from_str(&contents).map_err(|source| AuthStoreError::Corrupt { - path: self.path.clone(), - source, - }) - } - - fn read_auth_file_if_exists(&self) -> Result { - if !self.path.exists() { - return Ok(AuthFile::default()); + match fs::read_to_string(&self.path) { + Ok(contents) => { + serde_json::from_str(&contents).map_err(|source| AuthStoreError::Corrupt { + path: self.path.clone(), + source, + }) + } + Err(source) if source.kind() == std::io::ErrorKind::NotFound => Ok(AuthFile::default()), + Err(source) => Err(AuthStoreError::Read { + path: self.path.clone(), + source, + }), } - self.read_auth_file() } #[cfg(unix)] diff --git a/lib/crates/fabro-client/src/client.rs b/lib/crates/fabro-client/src/client.rs index 57098e055..3cc165c28 100644 --- a/lib/crates/fabro-client/src/client.rs +++ b/lib/crates/fabro-client/src/client.rs @@ -1294,7 +1294,7 @@ fn connect_target_transport( Ok((http_client, "http://fabro".to_string())) } -fn apply_bearer_token_auth( +pub fn apply_bearer_token_auth( builder: fabro_http::HttpClientBuilder, token: &str, ) -> Result { @@ -1307,7 +1307,7 @@ fn apply_bearer_token_auth( Ok(builder.default_headers(headers)) } -fn ensure_refresh_target_transport(target: &ServerTarget) -> Result<()> { +pub fn ensure_refresh_target_transport(target: &ServerTarget) -> Result<()> { match target.loopback_classification()? { LoopbackClassification::Https | LoopbackClassification::LoopbackHttp diff --git a/lib/crates/fabro-client/src/lib.rs b/lib/crates/fabro-client/src/lib.rs index 248987500..e8a78d380 100644 --- a/lib/crates/fabro-client/src/lib.rs +++ b/lib/crates/fabro-client/src/lib.rs @@ -1,7 +1,4 @@ //! Typed HTTP client for the Fabro API. -//! -//! This crate hosts the reusable client and auth/session plumbing that was -//! previously embedded in `fabro-cli`. pub mod auth_store; pub mod client; @@ -13,14 +10,16 @@ pub mod sse; pub mod target; pub use auth_store::{AuthEntry, AuthStore, AuthStoreError, LockError, StoredSubject}; -pub use client::{Client, RunEventStream, TransportConnector}; +pub use client::{ + Client, RunEventStream, TransportConnector, apply_bearer_token_auth, + ensure_refresh_target_transport, +}; pub use credential::{Credential, CredentialFallback}; pub use error::{ ApiError, ApiFailure, StructuredApiError, classify_api_error, classify_http_response, convert_type, is_not_found_error, map_api_error, parse_error_response_value, raw_response_failure_error, }; -pub use fabro_api::types; pub use loopback::{LoopbackClassification, TargetSchemeError}; pub use session::OAuthSession; pub use target::ServerTarget; diff --git a/lib/crates/fabro-client/src/loopback.rs b/lib/crates/fabro-client/src/loopback.rs index 20a28b900..f980c9f43 100644 --- a/lib/crates/fabro-client/src/loopback.rs +++ b/lib/crates/fabro-client/src/loopback.rs @@ -122,8 +122,6 @@ fn ip_is_loopback(ip: &IpAddr) -> bool { #[cfg(test)] mod tests { - use std::path::PathBuf; - use super::LoopbackClassification; use crate::target::ServerTarget;