From 5047f0bde47ecf3fba049f131671520300c300ba Mon Sep 17 00:00:00 2001 From: Simon Gurcke Date: Mon, 26 Oct 2026 00:03:36 +1000 Subject: [PATCH] Use device flow for browser auth Replace the loopback callback server in `apitally auth` with a device authorization flow. The CLI creates a login session via the public API, opens the verification URL, prints the code to confirm in the browser, and polls for the API key once the user approves. Remove the hidden --app-url option and the paste API key prompt. --- AGENTS.md | 28 +-- README.md | 2 +- skills/apitally-cli/references/commands.md | 2 +- src/auth.rs | 272 ++++++++------------- src/main.rs | 26 +- src/utils.rs | 13 + 6 files changed, 133 insertions(+), 210 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 36b693c..ea41496 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -9,7 +9,7 @@ ``` src/ main.rs Entry point, CLI argument parsing (clap), command dispatch - auth.rs Authentication config (load/save/resolve) + auth command + auth.rs Authentication config (load/save/resolve) + auth command (device flow via /v1/cli-auth/*) whoami.rs Whoami command (auth check, team info) apps.rs Apps command (fetch, DB write) consumers.rs Consumers command (paginated fetch, DB write) @@ -41,18 +41,18 @@ skills/ ## CLI Subcommands -| Subcommand | Data source | -| ----------------- | --------------------------------------------------- | -| `auth` | — | -| `whoami` | `GET /v1/team` | -| `apps` | `GET /v1/apps` | -| `consumers` | `GET /v1/apps/{app_id}/consumers` | -| `endpoints` | `GET /v1/apps/{app_id}/endpoints` | -| `metrics` | `POST /v1/apps/{app_id}/metrics` | -| `request-logs` | `POST /v1/apps/{app_id}/request-logs` | -| `traces` | `POST /v1/apps/{app_id}/traces` | -| `request-details` | `GET /v1/apps/{app_id}/request-logs/{request_uuid}` | -| `sql` | Local DuckDB | +| Subcommand | Data source | +| ----------------- | ------------------------------------------------------- | +| `auth` | `POST /v1/cli-auth/sessions`, `POST /v1/cli-auth/token` | +| `whoami` | `GET /v1/team` | +| `apps` | `GET /v1/apps` | +| `consumers` | `GET /v1/apps/{app_id}/consumers` | +| `endpoints` | `GET /v1/apps/{app_id}/endpoints` | +| `metrics` | `POST /v1/apps/{app_id}/metrics` | +| `request-logs` | `POST /v1/apps/{app_id}/request-logs` | +| `traces` | `POST /v1/apps/{app_id}/traces` | +| `request-details` | `GET /v1/apps/{app_id}/request-logs/{request_uuid}` | +| `sql` | Local DuckDB | ## Authentication with API @@ -94,7 +94,7 @@ Triggered by publishing a GitHub release. Two jobs: ### Module `run()` functions -Each command module exposes a `pub fn run(...)` that does all the work: resolve auth, call the API, write output. The last parameter is `impl Write` (stdout in production, `Vec` in tests), except `auth::run` which takes `&mut impl Read` (stdin) instead. Data goes to the writer (stdout), human messages and progress go to stderr. +Each command module exposes a `pub fn run(...)` that does all the work: resolve auth, call the API, write output. The last parameter is `impl Write` (stdout in production, `Vec` in tests), except `auth::run`, which writes only to stderr and takes the `auth.json` path instead. Data goes to the writer (stdout), human messages and progress go to stderr. ### Error handling and exit codes diff --git a/README.md b/README.md index f1740d7..1af2ec2 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Run the `auth` command to authenticate the CLI: npx @apitally/cli auth ``` -This opens a browser-based auth flow where you log in to the Apitally dashboard and select a team. A newly created API key is then passed back to the CLI and saved to `~/.apitally/auth.json`. +This opens the Apitally dashboard in your browser, where you confirm the code shown in the terminal, select a team and choose the API key's scope. Once you approve the login, the CLI fetches a newly created API key and saves it to `~/.apitally/auth.json`. If you already have an API key, you can provide it directly: diff --git a/skills/apitally-cli/references/commands.md b/skills/apitally-cli/references/commands.md index c5716ba..50a2836 100644 --- a/skills/apitally-cli/references/commands.md +++ b/skills/apitally-cli/references/commands.md @@ -12,7 +12,7 @@ Datetime flags (e.g. `--since`, `--until`, `--requests-since`) accept ISO 8601 s npx @apitally/cli auth [--api-key ] ``` -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. +Opens a browser-based auth flow where the user logs in to the Apitally dashboard, confirms the code shown in the terminal, and picks a team and a scope (read-only or read & write). The CLI then fetches the newly created API key. 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. diff --git a/src/auth.rs b/src/auth.rs index e0bee15..6ad0938 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,17 +1,14 @@ use std::fs; -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 std::time::{Duration, Instant}; use anyhow::{Context, Result}; use serde::{Deserialize, Serialize}; use regex::Regex; -use crate::utils::{ansi, auth_err}; +use crate::utils::{ansi, api_post_without_key, auth_err}; const DEFAULT_API_BASE_URL: &str = "https://api.apitally.io"; @@ -22,6 +19,20 @@ pub struct AuthConfig { pub api_base_url: Option, } +#[derive(Deserialize)] +struct CliAuthSessionResponse { + device_code: String, + user_code: String, + verification_url: String, + expires_in: u64, + interval: u64, +} + +#[derive(Deserialize)] +struct CliAuthTokenResponse { + api_key: String, +} + pub fn auth_file_path() -> Result { let home = dirs::home_dir().context("could not determine home directory")?; Ok(home.join(".apitally").join("auth.json")) @@ -105,13 +116,11 @@ 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: Option>, ) -> Result<()> { let api_key = match api_key { Some(key) => key, - None => browser_auth(app_url, input)?, + None => browser_auth(&pick_api_base_url(api_base_url.as_deref(), None))?, }; validate_api_key(&api_key)?; save_auth_file( @@ -125,111 +134,52 @@ pub fn run( Ok(()) } -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(); - let url = format!("{app_url}/cli-auth?callback_port={port}"); - - #[cfg(not(test))] - let _ = open::that(&url); - - eprintln!("Opening browser with URL: {url}\n"); - eprintln!("Complete the auth flow in the browser."); - if input.is_some() { - eprint!("Or paste your API key and press Enter: "); - } - io::stderr().flush()?; - - let (tx, rx) = mpsc::channel(); - - 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)).map_err(|_| { - eprintln!("\n"); - auth_err("authentication timed out") - })?; - Ok(api_key) -} - -fn read_stdin(tx: mpsc::Sender, input: Box) { - let mut line = String::new(); - if io::BufReader::new(input).read_line(&mut line).is_ok() { - let key = line.trim().to_string(); - if !key.is_empty() { - eprintln!(); - let _ = tx.send(key); - } - } -} - -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, app_url) { - eprintln!("\n"); - let _ = tx.send(api_key); - return; - } - } -} +fn browser_auth(api_base_url: &str) -> Result { + let url = format!("{api_base_url}/v1/cli-auth/sessions"); + let mut response = api_post_without_key(&url, &serde_json::json!({}))?; + let session: CliAuthSessionResponse = response.body_mut().read_json()?; -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 = 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" - ); - stream.write_all(response.as_bytes()).ok(); - return None; - } + eprintln!("Opening browser with URL: {}\n", session.verification_url); + eprintln!( + "Confirm that the browser shows this code: {}\n", + session.user_code + ); - 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 = format!( - "HTTP/1.1 200 OK\r\n\ - Access-Control-Allow-Origin: {app_url}\r\n\ - Content-Length: 0\r\n\ - \r\n" - ); - stream.write_all(response.as_bytes()).ok(); - return Some(api_key); + #[cfg(not(test))] + let _ = open::that(&session.verification_url); + + let token_url = format!("{api_base_url}/v1/cli-auth/token"); + let body = serde_json::json!({ "device_code": session.device_code }); + let deadline = Instant::now() + Duration::from_secs(session.expires_in); + while Instant::now() < deadline { + thread::sleep(Duration::from_secs(session.interval)); + let Ok(mut response) = ureq::post(&token_url) + .config() + .http_status_as_error(false) + .timeout_connect(Some(Duration::from_secs(5))) + .timeout_recv_response(Some(Duration::from_secs(15))) + .build() + .send_json(&body) + else { + continue; + }; + match response.status().as_u16() { + 200 => { + return Ok(response + .body_mut() + .read_json::()? + .api_key); + } + 429 => {} + 400..=499 => { + return Err(auth_err( + "authentication expired, run `apitally auth` again", + )); + } + _ => {} } - - 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" - ); - stream.write_all(response.as_bytes()).ok(); - return None; } - - None + Err(auth_err("authentication timed out")) } #[cfg(test)] @@ -295,9 +245,7 @@ mod tests { run( Some(TEST_API_KEY.into()), Some("https://custom.api".into()), - "https://app.apitally.io", &path, - None, ) .unwrap(); let config = load_auth_file(&path).unwrap().unwrap(); @@ -306,69 +254,53 @@ mod tests { } #[test] - fn test_run_with_stdin() { + fn test_run_with_polling() { + let mut server = mockito::Server::new(); + let sessions_mock = server + .mock("POST", "/v1/cli-auth/sessions") + .with_status(201) + .with_header("content-type", "application/json") + .with_body( + serde_json::json!({ + "device_code": "test-device-code", + "user_code": "WDJB-MJHT", + "verification_url": "https://app.apitally.io/cli-auth?code=WDJB-MJHT", + "expires_in": 600, + "interval": 0, + }) + .to_string(), + ) + .create(); + let pending_mock = server + .mock("POST", "/v1/cli-auth/token") + .match_body(mockito::Matcher::Json( + serde_json::json!({"device_code": "test-device-code"}), + )) + .with_status(202) + .with_body("{}") + .expect(1) + .create(); + let approved_mock = server + .mock("POST", "/v1/cli-auth/token") + .match_body(mockito::Matcher::Json( + serde_json::json!({"device_code": "test-device-code"}), + )) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(format!(r#"{{"api_key":"{TEST_API_KEY}"}}"#)) + .expect(1) + .create(); + let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("auth.json"); - 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(); + run(None, Some(server.url()), &path).unwrap(); + sessions_mock.assert(); + pending_mock.assert(); + approved_mock.assert(); + let config = load_auth_file(&path).unwrap().unwrap(); assert_eq!(config.api_key, TEST_API_KEY); - assert!(config.api_base_url.is_none()); - } - - #[test] - fn test_run_with_callback() { - let listener = TcpListener::bind("127.0.0.1:0").unwrap(); - let port = listener.local_addr().unwrap().port(); - - let (tx, rx) = mpsc::channel(); - 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(); - 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(&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 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(), - 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, TEST_API_KEY); + assert_eq!(config.api_base_url, Some(server.url())); } #[test] diff --git a/src/main.rs b/src/main.rs index 4252879..7e0d277 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,7 @@ mod traces; mod utils; mod whoami; -use std::io::{IsTerminal, Read}; +use std::io::Read; use std::path::PathBuf; use anyhow::Result; @@ -48,15 +48,6 @@ 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 @@ -495,20 +486,7 @@ fn exit_code(err: &anyhow::Error) -> i32 { fn run(cli: Cli) -> Result<()> { match cli.command { - Command::Auth { api, app_url } => { - 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()?, - input, - ) - } + Command::Auth { api } => auth::run(api.api_key, api.api_base_url, &auth::auth_file_path()?), Command::Whoami { api } => whoami::run( api.api_key.as_deref(), api.api_base_url.as_deref(), diff --git a/src/utils.rs b/src/utils.rs index 8e29c92..6699012 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -143,6 +143,19 @@ pub fn api_post(url: &str, api_key: &str, body: &serde_json::Value) -> Result Result> { + let mut response = ureq::post(url) + .config() + .http_status_as_error(false) + .timeout_connect(Some(Duration::from_secs(5))) + .timeout_recv_response(Some(Duration::from_secs(15))) + .build() + .send_json(body) + .map_err(|e| api_err(e.to_string()))?; + check_response(&mut response)?; + Ok(response) +} + fn check_response(response: &mut Response) -> Result<()> { let status = response.status().as_u16(); if status >= 400 {