From fabd1f130ce4257e6bf83a60102ad61756b25174 Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Wed, 15 Apr 2026 15:56:03 +1000 Subject: [PATCH 1/5] Add browser-based auth flow --- Cargo.lock | 37 +++++ Cargo.toml | 1 + skills/apitally-cli/SKILL.md | 4 +- skills/apitally-cli/references/commands.md | 4 +- src/auth.rs | 160 ++++++++++++++++++--- src/main.rs | 19 ++- src/utils.rs | 9 ++ 7 files changed, 204 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 49844d7..ec6eb35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -118,6 +118,7 @@ dependencies = [ "dirs", "duckdb", "mockito", + "open", "serde", "serde_json", "tempfile", @@ -1254,6 +1255,25 @@ dependencies = [ "serde", ] +[[package]] +name = "is-docker" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" +dependencies = [ + "once_cell", +] + +[[package]] +name = "is-wsl" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" +dependencies = [ + "is-docker", + "once_cell", +] + [[package]] name = "is_terminal_polyfill" version = "1.70.2" @@ -1584,6 +1604,17 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" +[[package]] +name = "open" +version = "5.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43bb73a7fa3799b198970490a51174027ba0d4ec504b03cd08caf513d40024bc" +dependencies = [ + "is-wsl", + "libc", + "pathdiff", +] + [[package]] name = "option-ext" version = "0.2.0" @@ -1613,6 +1644,12 @@ dependencies = [ "windows-link", ] +[[package]] +name = "pathdiff" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "df94ce210e5bc13cb6651479fa48d14f601d9858cfe0467f43ae157023b938d3" + [[package]] name = "percent-encoding" version = "2.3.2" diff --git a/Cargo.toml b/Cargo.toml index 58081dc..7cf898d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ arrow-ipc = { version = "=58.1.0", features = ["lz4"] } clap = { version = "4", features = ["derive", "env"] } dirs = "6" duckdb = { version = "=1.10501.0", features = ["bundled", "appender-arrow"] } +open = "5" serde = { version = "1", features = ["derive"] } serde_json = "1" ureq = { version = "3", features = ["json"] } diff --git a/skills/apitally-cli/SKILL.md b/skills/apitally-cli/SKILL.md index d328ff0..94cc114 100644 --- a/skills/apitally-cli/SKILL.md +++ b/skills/apitally-cli/SKILL.md @@ -21,7 +21,7 @@ Run commands with `npx` (no install needed): npx @apitally/cli [--api-key ] ``` -A team-scoped API key is required to use the CLI. The `auth` command writes the provided API key to `~/.apitally/auth.json`. It's then used by all subsequent commands unless overridden by the `--api-key` flag. +A team-scoped API key is required to use the CLI. The `auth` command saves an API key to `~/.apitally/auth.json`, which is then used by all subsequent commands unless overridden by the `--api-key` flag. All commands output NDJSON to stdout by default. With `--db`, data is written to a DuckDB database instead (`~/.apitally/data.duckdb` by default), enabling SQL queries via the `sql` command. @@ -50,7 +50,7 @@ All commands are run via `npx @apitally/cli `. For full details, see [r ## Investigation Workflow -1. **Check authentication** — run `npx @apitally/cli whoami`. If it fails, ask the user to run `npx @apitally/cli auth` to set their API key. Explain that API keys can be created in the Apitally dashboard under Settings > API keys (https://app.apitally.io/settings/api-keys). +1. **Check authentication** — run `npx @apitally/cli whoami`. If it fails, ask the user to run `npx @apitally/cli auth` to authenticate. 2. **Identify the app** — run `npx @apitally/cli apps` to list apps and get their IDs. If there is more than one app, and the correct app can't be inferred from the user's messages, ask the user which app they mean. Use the app ID consistently for all commands and SQL `WHERE` conditions throughout the investigation. diff --git a/skills/apitally-cli/references/commands.md b/skills/apitally-cli/references/commands.md index a643336..359c8be 100644 --- a/skills/apitally-cli/references/commands.md +++ b/skills/apitally-cli/references/commands.md @@ -10,7 +10,9 @@ Commands that accept a `--db` flag use `~/.apitally/data.duckdb` as the default npx @apitally/cli auth [--api-key ] ``` -Configure API key interactively or by providing a key directly. Saves API key to `~/.apitally/auth.json`. +Opens a browser-based auth flow where the user logs in to the Apitally dashboard and selects a team. A newly created API key is then passed back to the CLI. The key is saved to `~/.apitally/auth.json` and used by all subsequent commands unless overridden by the `--api-key` flag. + +If `--api-key` is provided, the key is saved directly without opening the browser. ## `whoami` diff --git a/src/auth.rs b/src/auth.rs index 7a0d447..c2ce727 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,11 +1,15 @@ use std::fs; -use std::io::{self, BufRead, Write}; +use std::io::{self, BufRead, Read, Write}; +use std::net::{TcpListener, TcpStream}; use std::path::{Path, PathBuf}; +use std::sync::mpsc; +use std::thread; +use std::time::Duration; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; -use crate::utils::auth_err; +use crate::utils::{ansi, auth_err}; const DEFAULT_API_BASE_URL: &str = "https://api.apitally.io"; @@ -91,12 +95,12 @@ pub fn resolve_api_base_url(api_base_url: Option<&str>) -> String { pub fn run( api_key: Option, api_base_url: Option, + app_url: &str, auth_file_path: &Path, - input: &mut impl io::Read, ) -> Result<()> { let api_key = match api_key { Some(key) => key, - None => prompt_api_key(input)?, + None => browser_auth(app_url)?, }; save_auth_file( auth_file_path, @@ -105,22 +109,98 @@ pub fn run( api_base_url, }, )?; - eprintln!("Authentication configured successfully."); + eprintln!("{}", ansi("1;32", "API key configured successfully.")); Ok(()) } -fn prompt_api_key(input: &mut impl io::Read) -> Result { - eprintln!("To get your API key, go to https://app.apitally.io/settings/api-keys"); - eprintln!(); - eprint!("API key: "); +fn browser_auth(app_url: &str) -> Result { + let listener = + TcpListener::bind("127.0.0.1:0").context("failed to start local callback server")?; + let port = listener.local_addr()?.port(); + + let url = format!("{app_url}/cli-auth?callback_port={port}"); + let _ = open::that(&url); + + eprintln!("Opening browser with URL: {url}\n"); + eprintln!("Complete the auth flow in the browser."); + eprint!("Or paste your API key and press Enter: "); io::stderr().flush()?; + + let (tx, rx) = mpsc::channel(); + + let tx_server = tx.clone(); + thread::spawn(move || run_callback_server(listener, tx_server)); + thread::spawn(move || read_stdin(tx)); + + let api_key = rx + .recv_timeout(Duration::from_secs(300)) + .map_err(|_| auth_err("authentication timed out"))?; + Ok(api_key) +} + +fn read_stdin(tx: mpsc::Sender) { let mut line = String::new(); - io::BufReader::new(input).read_line(&mut line)?; - let key = line.trim().to_string(); - if key.is_empty() { - return Err(auth_err("API key cannot be empty")); + if io::stdin().lock().read_line(&mut line).is_ok() { + let key = line.trim().to_string(); + if !key.is_empty() { + eprintln!(); + let _ = tx.send(key); + } } - Ok(key) +} + +fn run_callback_server(listener: TcpListener, tx: mpsc::Sender) { + listener.set_nonblocking(false).ok(); + while let Ok((mut stream, _)) = listener.accept() { + if let Some(api_key) = handle_callback_request(&mut stream) { + eprintln!("\n"); + let _ = tx.send(api_key); + return; + } + } +} + +fn handle_callback_request(stream: &mut TcpStream) -> Option { + let mut buf = [0u8; 4096]; + let n = stream.read(&mut buf).ok()?; + let request = std::str::from_utf8(&buf[..n]).ok()?; + + if request.starts_with("OPTIONS ") { + let response = "HTTP/1.1 204 No Content\r\n\ + Access-Control-Allow-Origin: *\r\n\ + Access-Control-Allow-Methods: POST\r\n\ + Access-Control-Allow-Headers: Content-Type\r\n\ + Content-Length: 0\r\n\ + \r\n"; + stream.write_all(response.as_bytes()).ok(); + return None; + } + + if request.starts_with("POST ") { + let api_key = request + .split("\r\n\r\n") + .nth(1) + .and_then(|body| serde_json::from_str::(body).ok()) + .and_then(|parsed| parsed["api_key"].as_str().map(String::from)); + + if let Some(api_key) = api_key { + let response = "HTTP/1.1 200 OK\r\n\ + Access-Control-Allow-Origin: *\r\n\ + Content-Length: 0\r\n\ + \r\n"; + stream.write_all(response.as_bytes()).ok(); + return Some(api_key); + } + + let response = "HTTP/1.1 400 Bad Request\r\n\ + Access-Control-Allow-Origin: *\r\n\ + Content-Length: 0\r\n\ + \r\n"; + stream.write_all(response.as_bytes()).ok(); + return None; + } + + None } #[cfg(test)] @@ -184,8 +264,8 @@ mod tests { run( Some("provided-key".into()), Some("https://custom.api".into()), + "https://app.apitally.io", &path, - &mut io::empty(), ) .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); @@ -194,14 +274,54 @@ mod tests { } #[test] - fn test_run_with_prompted_key() { + fn test_run_with_callback() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); - let mut input = io::Cursor::new(b"prompted-key\n"); - run(None, None, &path, &mut input).unwrap(); + + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = listener.local_addr().unwrap().port(); + + let (tx, rx) = mpsc::channel(); + thread::spawn(move || run_callback_server(listener, tx)); + + // Send CORS preflight + let mut stream = TcpStream::connect(format!("127.0.0.1:{port}")).unwrap(); + stream + .write_all(b"OPTIONS /callback HTTP/1.1\r\nOrigin: https://app.apitally.io\r\n\r\n") + .unwrap(); + let mut response = vec![0u8; 1024]; + let n = stream.read(&mut response).unwrap(); + let response_str = std::str::from_utf8(&response[..n]).unwrap(); + assert!(response_str.contains("204")); + assert!(response_str.contains("Access-Control-Allow-Origin: *")); + + // Send callback POST + let body = r#"{"api_key":"callback-key"}"#; + let request = format!( + "POST /callback HTTP/1.1\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + body.len(), + body + ); + let mut stream = TcpStream::connect(format!("127.0.0.1:{port}")).unwrap(); + stream.write_all(request.as_bytes()).unwrap(); + let mut response = vec![0u8; 1024]; + let n = stream.read(&mut response).unwrap(); + let response_str = std::str::from_utf8(&response[..n]).unwrap(); + assert!(response_str.contains("200")); + + let api_key = rx.recv_timeout(Duration::from_secs(5)).unwrap(); + assert_eq!(api_key, "callback-key"); + + save_auth_file( + &path, + &AuthConfig { + api_key, + api_base_url: None, + }, + ) + .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); - assert_eq!(config.api_key, "prompted-key"); - assert!(config.api_base_url.is_none()); + assert_eq!(config.api_key, "callback-key"); } #[test] diff --git a/src/main.rs b/src/main.rs index e4f9cd1..587448a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -47,6 +47,15 @@ enum Command { Auth { #[command(flatten)] api: ApiArgs, + + /// URL of the Apitally dashboard (for browser-based auth) + #[arg( + long, + env = "APITALLY_APP_URL", + default_value = "https://app.apitally.io", + hide = true + )] + app_url: String, }, /// Show the authenticated team @@ -302,11 +311,7 @@ enum Command { fn main() { let cli = Cli::parse(); if let Err(err) = run(cli) { - if std::io::stderr().is_terminal() { - eprintln!("\x1b[1;31merror:\x1b[0m {err:#}"); - } else { - eprintln!("error: {err:#}"); - } + eprintln!("{} {err:#}", utils::ansi("1;31", "error:")); std::process::exit(exit_code(&err)); } } @@ -326,7 +331,7 @@ fn exit_code(err: &anyhow::Error) -> i32 { fn run(cli: Cli) -> Result<()> { match cli.command { - Command::Auth { api } => { + Command::Auth { api, app_url } => { if api.api_key.is_none() && !std::io::stdin().is_terminal() { return Err(utils::auth_err( "no API key provided. Use --api-key or set APITALLY_API_KEY", @@ -335,8 +340,8 @@ fn run(cli: Cli) -> Result<()> { auth::run( api.api_key, api.api_base_url, + &app_url, &auth::auth_file_path()?, - &mut std::io::stdin(), ) } Command::Whoami { api } => whoami::run( diff --git a/src/utils.rs b/src/utils.rs index 5fc364e..9110f18 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -1,3 +1,4 @@ +use std::io::IsTerminal; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -34,6 +35,14 @@ pub fn api_err(msg: impl Into) -> anyhow::Error { CliError::Api(msg.into()).into() } +pub fn ansi(code: &str, text: impl std::fmt::Display) -> String { + if std::io::stderr().is_terminal() { + format!("\x1b[{code}m{text}\x1b[0m") + } else { + text.to_string() + } +} + pub fn default_db_path() -> Result { let home = dirs::home_dir().context("could not determine home directory")?; Ok(home.join(".apitally").join("data.duckdb")) From 37bcfd529c358002403e30e6c3992fe2f0f301f3 Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Wed, 15 Apr 2026 16:17:07 +1000 Subject: [PATCH 2/5] Add test --- src/auth.rs | 28 +++++++++++++++++++++------- src/main.rs | 1 + 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index c2ce727..d43e445 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -97,10 +97,11 @@ pub fn run( api_base_url: Option, app_url: &str, auth_file_path: &Path, + input: Box, ) -> Result<()> { let api_key = match api_key { Some(key) => key, - None => browser_auth(app_url)?, + None => browser_auth(app_url, input)?, }; save_auth_file( auth_file_path, @@ -113,12 +114,13 @@ pub fn run( Ok(()) } -fn browser_auth(app_url: &str) -> Result { +fn browser_auth(app_url: &str, input: Box) -> Result { let listener = TcpListener::bind("127.0.0.1:0").context("failed to start local callback server")?; let port = listener.local_addr()?.port(); - let url = format!("{app_url}/cli-auth?callback_port={port}"); + + #[cfg(not(test))] let _ = open::that(&url); eprintln!("Opening browser with URL: {url}\n"); @@ -130,7 +132,7 @@ fn browser_auth(app_url: &str) -> Result { let tx_server = tx.clone(); thread::spawn(move || run_callback_server(listener, tx_server)); - thread::spawn(move || read_stdin(tx)); + thread::spawn(move || read_stdin(tx, input)); let api_key = rx .recv_timeout(Duration::from_secs(300)) @@ -138,9 +140,9 @@ fn browser_auth(app_url: &str) -> Result { Ok(api_key) } -fn read_stdin(tx: mpsc::Sender) { +fn read_stdin(tx: mpsc::Sender, input: Box) { let mut line = String::new(); - if io::stdin().lock().read_line(&mut line).is_ok() { + if io::BufReader::new(input).read_line(&mut line).is_ok() { let key = line.trim().to_string(); if !key.is_empty() { eprintln!(); @@ -258,7 +260,7 @@ mod tests { } #[test] - fn test_run_with_provided_key() { + fn test_run_with_api_key_flag() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); run( @@ -266,6 +268,7 @@ mod tests { Some("https://custom.api".into()), "https://app.apitally.io", &path, + Box::new(io::empty()), ) .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); @@ -273,6 +276,17 @@ mod tests { assert_eq!(config.api_base_url.as_deref(), Some("https://custom.api")); } + #[test] + fn test_run_with_stdin() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("auth.json"); + let input = Box::new(io::Cursor::new(b"stdin-key\n".to_vec())); + run(None, None, "https://app.apitally.io", &path, input).unwrap(); + let config = load_auth_file(&path).unwrap().unwrap(); + assert_eq!(config.api_key, "stdin-key"); + assert!(config.api_base_url.is_none()); + } + #[test] fn test_run_with_callback() { let dir = tempfile::tempdir().unwrap(); diff --git a/src/main.rs b/src/main.rs index 587448a..c225517 100644 --- a/src/main.rs +++ b/src/main.rs @@ -342,6 +342,7 @@ fn run(cli: Cli) -> Result<()> { api.api_base_url, &app_url, &auth::auth_file_path()?, + Box::new(std::io::stdin()), ) } Command::Whoami { api } => whoami::run( From a18e64e8d2ec14d4337cf07b0246d0178f8fa0f1 Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Wed, 15 Apr 2026 16:30:23 +1000 Subject: [PATCH 3/5] Add API key validation --- Cargo.lock | 1 + Cargo.toml | 1 + src/auth.rs | 32 ++++++++++++++++++++++++++++---- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec6eb35..41417a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -119,6 +119,7 @@ dependencies = [ "duckdb", "mockito", "open", + "regex", "serde", "serde_json", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index 7cf898d..cc67620 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ clap = { version = "4", features = ["derive", "env"] } dirs = "6" duckdb = { version = "=1.10501.0", features = ["bundled", "appender-arrow"] } open = "5" +regex = "1" serde = { version = "1", features = ["derive"] } serde_json = "1" ureq = { version = "3", features = ["json"] } diff --git a/src/auth.rs b/src/auth.rs index d43e445..211508a 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -9,6 +9,8 @@ use std::time::Duration; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; +use regex::Regex; + use crate::utils::{ansi, auth_err}; const DEFAULT_API_BASE_URL: &str = "https://api.apitally.io"; @@ -78,6 +80,14 @@ fn pick_api_base_url(api_base_url: Option<&str>, config: Option<&AuthConfig>) -> DEFAULT_API_BASE_URL.to_string() } +fn validate_api_key(api_key: &str) -> Result<()> { + let re = Regex::new(r"^[a-zA-Z0-9]{7}\.[a-zA-Z0-9]{32}$").unwrap(); + if !re.is_match(api_key) { + return Err(auth_err("invalid API key format")); + } + Ok(()) +} + /// Resolve API key with precedence: --api-key flag / APITALLY_API_KEY env (via clap) > auth.json pub fn resolve_api_key(api_key: Option<&str>) -> Result { let config = load_auth_file(&auth_file_path()?)?; @@ -103,6 +113,7 @@ pub fn run( Some(key) => key, None => browser_auth(app_url, input)?, }; + validate_api_key(&api_key)?; save_auth_file( auth_file_path, &AuthConfig { @@ -209,6 +220,8 @@ fn handle_callback_request(stream: &mut TcpStream) -> Option { mod tests { use super::*; + const TEST_API_KEY: &str = "aBcDeFg.01234567890123456789012345678901"; + #[test] fn test_save_and_load_config() { let dir = tempfile::tempdir().unwrap(); @@ -264,7 +277,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); run( - Some("provided-key".into()), + Some(TEST_API_KEY.into()), Some("https://custom.api".into()), "https://app.apitally.io", &path, @@ -272,7 +285,7 @@ mod tests { ) .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); - assert_eq!(config.api_key, "provided-key"); + assert_eq!(config.api_key, TEST_API_KEY); assert_eq!(config.api_base_url.as_deref(), Some("https://custom.api")); } @@ -280,10 +293,10 @@ mod tests { fn test_run_with_stdin() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); - let input = Box::new(io::Cursor::new(b"stdin-key\n".to_vec())); + let input = Box::new(io::Cursor::new(format!("{TEST_API_KEY}\n").into_bytes())); run(None, None, "https://app.apitally.io", &path, input).unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); - assert_eq!(config.api_key, "stdin-key"); + assert_eq!(config.api_key, TEST_API_KEY); assert!(config.api_base_url.is_none()); } @@ -338,6 +351,17 @@ mod tests { assert_eq!(config.api_key, "callback-key"); } + #[test] + fn test_validate_api_key() { + assert!(validate_api_key(TEST_API_KEY).is_ok()); + assert!(validate_api_key("short.01234567890123456789012345678901").is_err()); + assert!(validate_api_key("aBcDeFg.short").is_err()); + assert!(validate_api_key("invalid-key").is_err()); + assert!(validate_api_key("").is_err()); + assert!(validate_api_key("abc!eFg.01234567890123456789012345678901").is_err()); + assert!(validate_api_key("aBcDeFg.0123456789012345678901234567890!").is_err()); + } + #[test] fn test_pick_api_base_url() { assert_eq!( From 3d218c096060e95bdf22659a52420cf317a414ad Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Wed, 15 Apr 2026 17:21:01 +1000 Subject: [PATCH 4/5] Fixes --- src/auth.rs | 99 +++++++++++++++++++++++++++++++---------------------- src/main.rs | 12 +++---- 2 files changed, 65 insertions(+), 46 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 211508a..f5e159e 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -107,7 +107,7 @@ pub fn run( api_base_url: Option, app_url: &str, auth_file_path: &Path, - input: Box, + input: Option>, ) -> Result<()> { let api_key = match api_key { Some(key) => key, @@ -125,7 +125,7 @@ pub fn run( Ok(()) } -fn browser_auth(app_url: &str, input: Box) -> Result { +fn browser_auth(app_url: &str, input: Option>) -> Result { let listener = TcpListener::bind("127.0.0.1:0").context("failed to start local callback server")?; let port = listener.local_addr()?.port(); @@ -136,14 +136,20 @@ fn browser_auth(app_url: &str, input: Box) -> Result { eprintln!("Opening browser with URL: {url}\n"); eprintln!("Complete the auth flow in the browser."); - eprint!("Or paste your API key and press Enter: "); + if input.is_some() { + eprint!("Or paste your API key and press Enter: "); + } io::stderr().flush()?; let (tx, rx) = mpsc::channel(); - let tx_server = tx.clone(); - thread::spawn(move || run_callback_server(listener, tx_server)); - thread::spawn(move || read_stdin(tx, input)); + if let Some(input) = input { + let tx_stdin = tx.clone(); + thread::spawn(move || read_stdin(tx_stdin, input)); + } + + let app_url = app_url.to_string(); + thread::spawn(move || run_callback_server(listener, tx, &app_url)); let api_key = rx .recv_timeout(Duration::from_secs(300)) @@ -162,10 +168,10 @@ fn read_stdin(tx: mpsc::Sender, input: Box) { } } -fn run_callback_server(listener: TcpListener, tx: mpsc::Sender) { +fn run_callback_server(listener: TcpListener, tx: mpsc::Sender, app_url: &str) { listener.set_nonblocking(false).ok(); while let Ok((mut stream, _)) = listener.accept() { - if let Some(api_key) = handle_callback_request(&mut stream) { + if let Some(api_key) = handle_callback_request(&mut stream, app_url) { eprintln!("\n"); let _ = tx.send(api_key); return; @@ -173,18 +179,23 @@ fn run_callback_server(listener: TcpListener, tx: mpsc::Sender) { } } -fn handle_callback_request(stream: &mut TcpStream) -> Option { +fn handle_callback_request(stream: &mut TcpStream, app_url: &str) -> Option { + stream + .set_read_timeout(Some(std::time::Duration::from_secs(1))) + .ok(); let mut buf = [0u8; 4096]; let n = stream.read(&mut buf).ok()?; let request = std::str::from_utf8(&buf[..n]).ok()?; if request.starts_with("OPTIONS ") { - let response = "HTTP/1.1 204 No Content\r\n\ - Access-Control-Allow-Origin: *\r\n\ + let response = format!( + "HTTP/1.1 204 No Content\r\n\ + Access-Control-Allow-Origin: {app_url}\r\n\ Access-Control-Allow-Methods: POST\r\n\ Access-Control-Allow-Headers: Content-Type\r\n\ Content-Length: 0\r\n\ - \r\n"; + \r\n" + ); stream.write_all(response.as_bytes()).ok(); return None; } @@ -197,18 +208,22 @@ fn handle_callback_request(stream: &mut TcpStream) -> Option { .and_then(|parsed| parsed["api_key"].as_str().map(String::from)); if let Some(api_key) = api_key { - let response = "HTTP/1.1 200 OK\r\n\ - Access-Control-Allow-Origin: *\r\n\ + let response = format!( + "HTTP/1.1 200 OK\r\n\ + Access-Control-Allow-Origin: {app_url}\r\n\ Content-Length: 0\r\n\ - \r\n"; + \r\n" + ); stream.write_all(response.as_bytes()).ok(); return Some(api_key); } - let response = "HTTP/1.1 400 Bad Request\r\n\ - Access-Control-Allow-Origin: *\r\n\ + let response = format!( + "HTTP/1.1 400 Bad Request\r\n\ + Access-Control-Allow-Origin: {app_url}\r\n\ Content-Length: 0\r\n\ - \r\n"; + \r\n" + ); stream.write_all(response.as_bytes()).ok(); return None; } @@ -281,7 +296,7 @@ mod tests { Some("https://custom.api".into()), "https://app.apitally.io", &path, - Box::new(io::empty()), + None, ) .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); @@ -293,8 +308,9 @@ mod tests { fn test_run_with_stdin() { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); - let input = Box::new(io::Cursor::new(format!("{TEST_API_KEY}\n").into_bytes())); - run(None, None, "https://app.apitally.io", &path, input).unwrap(); + let input: Box = + Box::new(io::Cursor::new(format!("{TEST_API_KEY}\n").into_bytes())); + run(None, None, "https://app.apitally.io", &path, Some(input)).unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); assert_eq!(config.api_key, TEST_API_KEY); assert!(config.api_base_url.is_none()); @@ -302,14 +318,13 @@ mod tests { #[test] fn test_run_with_callback() { - let dir = tempfile::tempdir().unwrap(); - let path = dir.path().join("auth.json"); - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let port = listener.local_addr().unwrap().port(); let (tx, rx) = mpsc::channel(); - thread::spawn(move || run_callback_server(listener, tx)); + let app_url = "https://app.apitally.io"; + let app_url_owned = app_url.to_string(); + thread::spawn(move || run_callback_server(listener, tx, &app_url_owned)); // Send CORS preflight let mut stream = TcpStream::connect(format!("127.0.0.1:{port}")).unwrap(); @@ -320,10 +335,25 @@ mod tests { let n = stream.read(&mut response).unwrap(); let response_str = std::str::from_utf8(&response[..n]).unwrap(); assert!(response_str.contains("204")); - assert!(response_str.contains("Access-Control-Allow-Origin: *")); + assert!(response_str.contains(&format!("Access-Control-Allow-Origin: {app_url}"))); + + // Send callback POST with invalid data + let body = r#"{"invalid":"data"}"#; + let request = format!( + "POST /callback HTTP/1.1\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + body.len(), + body + ); + let mut stream = TcpStream::connect(format!("127.0.0.1:{port}")).unwrap(); + stream.write_all(request.as_bytes()).unwrap(); + let mut response = vec![0u8; 1024]; + let n = stream.read(&mut response).unwrap(); + let response_str = std::str::from_utf8(&response[..n]).unwrap(); + assert!(response_str.contains("400")); + assert!(response_str.contains(&format!("Access-Control-Allow-Origin: {app_url}"))); - // Send callback POST - let body = r#"{"api_key":"callback-key"}"#; + // Send callback POST with valid api_key + let body = format!(r#"{{"api_key":"{TEST_API_KEY}"}}"#); let request = format!( "POST /callback HTTP/1.1\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", body.len(), @@ -337,18 +367,7 @@ mod tests { assert!(response_str.contains("200")); let api_key = rx.recv_timeout(Duration::from_secs(5)).unwrap(); - assert_eq!(api_key, "callback-key"); - - save_auth_file( - &path, - &AuthConfig { - api_key, - api_base_url: None, - }, - ) - .unwrap(); - let config = load_auth_file(&path).unwrap().unwrap(); - assert_eq!(config.api_key, "callback-key"); + assert_eq!(api_key, TEST_API_KEY); } #[test] diff --git a/src/main.rs b/src/main.rs index c225517..8125cb9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -332,17 +332,17 @@ fn exit_code(err: &anyhow::Error) -> i32 { fn run(cli: Cli) -> Result<()> { match cli.command { Command::Auth { api, app_url } => { - if api.api_key.is_none() && !std::io::stdin().is_terminal() { - return Err(utils::auth_err( - "no API key provided. Use --api-key or set APITALLY_API_KEY", - )); - } + let input = if std::io::stdin().is_terminal() { + Some(Box::new(std::io::stdin()) as Box) + } else { + None + }; auth::run( api.api_key, api.api_base_url, &app_url, &auth::auth_file_path()?, - Box::new(std::io::stdin()), + input, ) } Command::Whoami { api } => whoami::run( From a5e920674b6535ff062c0ef13d3d5343173391d6 Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Wed, 15 Apr 2026 18:57:05 +1000 Subject: [PATCH 5/5] Fix --- src/auth.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index f5e159e..e0bee15 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -151,9 +151,10 @@ fn browser_auth(app_url: &str, input: Option>) -> Result