diff --git a/lib/crates/fabro-cli/src/server_client.rs b/lib/crates/fabro-cli/src/server_client.rs index 6cfd97713..e4eab7702 100644 --- a/lib/crates/fabro-cli/src/server_client.rs +++ b/lib/crates/fabro-cli/src/server_client.rs @@ -1,4 +1,5 @@ use std::collections::VecDeque; +use std::net::IpAddr; use std::num::NonZeroU64; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -90,7 +91,7 @@ pub(crate) async fn connect_server(storage_dir: &Path) -> Result Result { if target.starts_with("http://") || target.starts_with("https://") { - connect_remote_api_client_bundle(target, None) + connect_remote_api_client_bundle(target, None, RemoteDevTokenAuth::Ambient) } else { let path = Path::new(target); if !path.is_absolute() { @@ -146,9 +147,11 @@ async fn connect_target_api_client_bundle( runtime: &LocalServerRuntime, ) -> Result { match target { - user_config::ServerTarget::HttpUrl { api_url, tls } => { - connect_remote_api_client_bundle(api_url, tls.as_ref()) - } + user_config::ServerTarget::HttpUrl { api_url, tls } => connect_remote_api_client_bundle( + api_url, + tls.as_ref(), + remote_dev_token_auth_for_target(api_url, &runtime.storage_dir), + ), user_config::ServerTarget::UnixSocket(path) => { if let Ok(client) = try_connect_unix_socket_api_client_bundle(path, Some(&runtime.storage_dir)).await @@ -168,12 +171,36 @@ async fn connect_target_api_client_bundle( } } +#[derive(Clone, Copy)] +enum RemoteDevTokenAuth<'a> { + None, + Storage(&'a Path), + AmbientLocalTarget, + Ambient, +} + fn connect_remote_api_client_bundle( api_url: &str, tls: Option<&user_config::ClientTlsSettings>, + dev_token_auth: RemoteDevTokenAuth<'_>, ) -> Result { - let http_client = user_config::build_server_client(tls)?; let normalized = normalize_remote_server_target(api_url); + let mut builder = user_config::build_server_client_builder(tls)?; + builder = match dev_token_auth { + RemoteDevTokenAuth::None => builder, + RemoteDevTokenAuth::Storage(storage_dir) => { + apply_dev_token_auth(builder.no_proxy(), Some(storage_dir))? + } + RemoteDevTokenAuth::AmbientLocalTarget => { + if remote_url_targets_local_host(&normalized) { + apply_dev_token_auth(builder.no_proxy(), None)? + } else { + builder + } + } + RemoteDevTokenAuth::Ambient => apply_dev_token_auth(builder, None)?, + }; + let http_client = builder.build()?; let client = fabro_api::Client::new_with_client(&normalized, http_client.clone()); Ok(ServerStoreClient { client, @@ -190,6 +217,73 @@ fn normalize_remote_server_target(api_url: &str) -> String { .to_string() } +fn remote_dev_token_auth_for_target<'a>( + api_url: &str, + storage_dir: &'a Path, +) -> RemoteDevTokenAuth<'a> { + if remote_url_matches_active_local_tcp_server(api_url, storage_dir) { + RemoteDevTokenAuth::Storage(storage_dir) + } else if remote_url_targets_local_host(api_url) { + RemoteDevTokenAuth::AmbientLocalTarget + } else { + RemoteDevTokenAuth::None + } +} + +fn remote_url_matches_active_local_tcp_server(api_url: &str, storage_dir: &Path) -> bool { + let Some(record) = record::active_server_record(storage_dir) else { + return false; + }; + let Bind::Tcp(bind_addr) = record.bind else { + return false; + }; + remote_url_matches_tcp_bind(api_url, bind_addr) +} + +fn remote_url_matches_tcp_bind(api_url: &str, bind_addr: std::net::SocketAddr) -> bool { + let Ok(url) = fabro_http::Url::parse(&normalize_remote_server_target(api_url)) else { + return false; + }; + if url.scheme() != "http" && url.scheme() != "https" { + return false; + } + let Some(port) = url.port_or_known_default() else { + return false; + }; + if port != bind_addr.port() { + return false; + } + let Some(host) = url.host_str() else { + return false; + }; + host_matches_bind(host, bind_addr.ip()) +} + +fn remote_url_targets_local_host(api_url: &str) -> bool { + let Ok(url) = fabro_http::Url::parse(&normalize_remote_server_target(api_url)) else { + return false; + }; + let Some(host) = url.host_str() else { + return false; + }; + host_is_local(host) +} + +fn host_matches_bind(host: &str, bind_ip: IpAddr) -> bool { + host.parse::() + .ok() + .is_some_and(|host_ip| host_ip == bind_ip) + || (host_is_local(host) && (bind_ip.is_loopback() || bind_ip.is_unspecified())) +} + +fn host_is_local(host: &str) -> bool { + host.eq_ignore_ascii_case("localhost") + || host + .parse::() + .ok() + .is_some_and(|ip| ip.is_loopback() || ip.is_unspecified()) +} + fn load_dev_token_if_available(storage_dir: Option<&Path>) -> Option { if let Some(token) = std::env::var("FABRO_DEV_TOKEN") .ok() @@ -1064,6 +1158,48 @@ mod tests { assert_eq!(loaded.as_deref(), Some(token)); } + + #[test] + fn remote_url_matches_tcp_bind_accepts_loopback_aliases() { + let bind_addr = "127.0.0.1:32276".parse().unwrap(); + + assert!(remote_url_matches_tcp_bind( + "http://127.0.0.1:32276/api/v1", + bind_addr + )); + assert!(remote_url_matches_tcp_bind( + "http://localhost:32276", + bind_addr + )); + assert!(!remote_url_matches_tcp_bind( + "http://127.0.0.1:32277", + bind_addr + )); + } + + #[test] + fn remote_url_matches_tcp_bind_accepts_loopback_target_for_unspecified_bind() { + let bind_addr = "0.0.0.0:32276".parse().unwrap(); + + assert!(remote_url_matches_tcp_bind( + "http://127.0.0.1:32276", + bind_addr + )); + assert!(remote_url_matches_tcp_bind( + "http://localhost:32276/api/v1", + bind_addr + )); + } + + #[test] + fn remote_url_targets_local_host_detects_local_http_urls() { + assert!(remote_url_targets_local_host("http://127.0.0.1:32276")); + assert!(remote_url_targets_local_host( + "http://localhost:32276/api/v1" + )); + assert!(remote_url_targets_local_host("http://0.0.0.0:32276")); + assert!(!remote_url_targets_local_host("https://example.com")); + } } fn is_not_found_error(err: &progenitor_client::Error) -> bool diff --git a/lib/crates/fabro-cli/src/user_config.rs b/lib/crates/fabro-cli/src/user_config.rs index 316f45919..4074284d9 100644 --- a/lib/crates/fabro-cli/src/user_config.rs +++ b/lib/crates/fabro-cli/src/user_config.rs @@ -197,11 +197,11 @@ pub(crate) fn cli_http_client_builder() -> fabro_http::HttpClientBuilder { fabro_http::HttpClientBuilder::new().user_agent(format!("fabro-cli/{FABRO_VERSION}")) } -pub(crate) fn build_server_client( +pub(crate) fn build_server_client_builder( tls: Option<&ClientTlsSettings>, -) -> anyhow::Result { +) -> anyhow::Result { let Some(tls) = tls else { - return Ok(cli_http_client_builder().build()?); + return Ok(cli_http_client_builder()); }; let cert_path = fabro_config::expand_tilde(&tls.cert); @@ -219,13 +219,16 @@ pub(crate) fn build_server_client( let identity = fabro_http::Identity::from_pem(&identity_pem)?; let ca_cert = fabro_http::Certificate::from_pem(&ca_pem)?; - let client = cli_http_client_builder() + Ok(cli_http_client_builder() .use_rustls_tls() .identity(identity) - .add_root_certificate(ca_cert) - .build()?; + .add_root_certificate(ca_cert)) +} - Ok(client) +pub(crate) fn build_server_client( + tls: Option<&ClientTlsSettings>, +) -> anyhow::Result { + Ok(build_server_client_builder(tls)?.build()?) } #[cfg(test)] diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index 9e9fd102f..bde099b90 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -35,6 +35,58 @@ fn help() { "); } +#[test] +fn ps_accepts_local_tcp_server_target() { + let context = test_context!(); + let storage_root = tempfile::tempdir_in("/tmp").unwrap(); + let storage_dir = storage_root.path().join("storage"); + std::fs::create_dir_all(&storage_dir).unwrap(); + + context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "start", "--dry-run", "--bind", "127.0.0.1"]) + .assert() + .success(); + + let status_output = context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "status", "--json"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let status_json: Value = serde_json::from_slice(&status_output).unwrap(); + let bind = status_json["bind"] + .as_str() + .expect("bind should be present"); + + let output = context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["ps", "-a", "--json", "--server", &format!("http://{bind}")]) + .output() + .expect("ps should run"); + + context + .command() + .env("FABRO_STORAGE_DIR", &storage_dir) + .args(["server", "stop"]) + .assert() + .success(); + + assert!( + output.status.success(), + "ps against local TCP target failed:\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + let runs: Vec = serde_json::from_slice(&output.stdout).unwrap(); + assert!(runs.is_empty(), "new local TCP server should have no runs"); +} + #[test] fn ps_default_excludes_non_running_runs() { let context = test_context!(); diff --git a/lib/crates/fabro-server/src/jwt_auth.rs b/lib/crates/fabro-server/src/jwt_auth.rs index eb5ab8575..f82b0f5c5 100644 --- a/lib/crates/fabro-server/src/jwt_auth.rs +++ b/lib/crates/fabro-server/src/jwt_auth.rs @@ -1094,6 +1094,32 @@ enabled = true assert_eq!(body["auth_method"], "cookie"); } + #[tokio::test] + async fn dev_token_cookie_subject_extracts_dev_token_auth_method() { + let app = subject_router(AuthMode::Strategies(vec![AuthStrategy::Cookie])); + + let mut req = Request::builder() + .uri("/subject") + .body(Body::empty()) + .unwrap(); + req.extensions_mut().insert(SessionCookie { + login: "dev".to_string(), + provider: "dev-token".to_string(), + name: "Development User".to_string(), + email: "dev@localhost".to_string(), + avatar_url: "/logo.svg".to_string(), + user_url: String::new(), + provider_id: None, + exp: 9_999_999_999, + }); + + let response = app.oneshot(req).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let body = response_json(response).await; + assert_eq!(body["login"], "dev"); + assert_eq!(body["auth_method"], "dev_token"); + } + #[tokio::test] async fn dev_token_strategy_accepts_valid_bearer() { let app = test_router(AuthMode::Strategies(vec![AuthStrategy::DevToken { diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index d3c34975e..f89e88514 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -154,7 +154,6 @@ fn apply_runtime_settings( fn router_web_enabled(settings: &ResolvedServerSettings) -> bool { settings.web.enabled - && settings.integrations.github.strategy != GithubIntegrationStrategy::GhCli } fn use_in_memory_store() -> bool { @@ -805,7 +804,7 @@ enabled = false } #[test] - fn gh_cli_strategy_forces_web_disabled_in_router_options() { + fn web_enabled_stays_enabled_without_github_app_mode() { let base = parse_settings( r#" _version = 1 @@ -820,7 +819,7 @@ strategy = "gh_cli" let resolved = resolve_server_settings(&base).expect("settings should resolve"); - assert!(!router_web_enabled(&resolved)); + assert!(router_web_enabled(&resolved)); } #[test] diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index f209b8889..3ee130291 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -921,15 +921,8 @@ pub fn build_router_with_options( }, ); - let mut router = Router::new().route("/health", get(health)); - if options.web_enabled { - router = router.layer(middleware::from_fn_with_state( - middleware_state, - cookie_and_demo_middleware, - )); - } - - router + let mut router = Router::new() + .route("/health", get(health)) .fallback_service(service_fn(move |req: axum_extract::Request| { let dispatch = dispatch.clone(); async move { @@ -947,8 +940,16 @@ pub fn build_router_with_options( Ok::<_, std::convert::Infallible>(StatusCode::NOT_FOUND.into_response()) } } - })) - .layer(trace_layer) + })); + + if options.web_enabled { + router = router.layer(middleware::from_fn_with_state( + middleware_state, + cookie_and_demo_middleware, + )); + } + + router.layer(trace_layer) } fn demo_routes() -> Router> { @@ -6210,12 +6211,15 @@ mod tests { use std::process::Stdio; use axum::body::Body; - use axum::http::Request; + use axum::http::{Request, header}; + use axum_extra::extract::cookie::Key; use fabro_interview::{AnswerValue, ControlInterviewer, Interviewer, Question, QuestionType}; use fabro_types::{InterviewQuestionRecord, InterviewQuestionType, RunBlobId, RunId, fixtures}; + use serde_json::json; use tower::ServiceExt; use super::*; + use crate::jwt_auth::AuthStrategy; const MINIMAL_DOT: &str = r#"digraph Test { graph [goal="Test"] @@ -7026,6 +7030,84 @@ slug = "fabro" assert!(body["run"]["provenance"]["subject"]["login"].is_null()); } + #[tokio::test] + async fn dev_token_web_login_authorizes_cookie_backed_api_requests() { + const DEV_TOKEN: &str = + "fabro_dev_abababababababababababababababababababababababababababababababab"; + + let state = create_test_app_state_with_session_key( + SettingsLayer::default(), + Some(Key::derive_from(b"server-test-session-key-0123456789")), + false, + ); + let app = build_router( + Arc::clone(&state), + AuthMode::Strategies(vec![ + AuthStrategy::DevToken { + token: DEV_TOKEN.to_string(), + }, + AuthStrategy::Cookie, + ]), + ); + + let login_response = app + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri("/auth/login/dev-token") + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(json!({ "token": DEV_TOKEN }).to_string())) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(login_response.status(), StatusCode::OK); + let session_cookie = login_response + .headers() + .get(header::SET_COOKIE) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.split(';').next()) + .expect("session cookie should be set") + .to_string(); + + let create_response = app + .clone() + .oneshot( + Request::builder() + .method("POST") + .uri(api("/runs")) + .header(header::CONTENT_TYPE, "application/json") + .header(header::COOKIE, &session_cookie) + .body(manifest_body(MINIMAL_DOT)) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(create_response.status(), StatusCode::CREATED); + let create_body = body_json(create_response.into_body()).await; + let run_id = create_body["id"].as_str().unwrap(); + + let state_response = app + .oneshot( + Request::builder() + .method("GET") + .uri(api(&format!("/runs/{run_id}/state"))) + .header(header::COOKIE, &session_cookie) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(state_response.status(), StatusCode::OK); + let state_body = body_json(state_response.into_body()).await; + assert_eq!( + state_body["run"]["provenance"]["subject"]["auth_method"], + "dev_token" + ); + assert_eq!(state_body["run"]["provenance"]["subject"]["login"], "dev"); + } + #[tokio::test] async fn create_run_persists_manifest_and_definition_blobs_without_bundle_file() { let state = create_app_state();