diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f1794fe..0d5695e 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -152,19 +152,16 @@ jobs: organization="$(sed -n 's/^RAWTREE_LOCAL_BOOTSTRAP_ORGANIZATION=//p' ../platform/.env.local)" cluster="$(sed -n 's/^RAWTREE_LOCAL_BOOTSTRAP_CLUSTER=//p' ../platform/.env.local)" email="$(sed -n 's/^RAWTREE_LOCAL_BOOTSTRAP_EMAIL=//p' ../platform/.env.local)" - password="$(sed -n 's/^RAWTREE_LOCAL_BOOTSTRAP_PASSWORD=//p' ../platform/.env.local)" - if [ -z "$token" ] || [ -z "$organization" ] || [ -z "$cluster" ] || [ -z "$email" ] || [ -z "$password" ]; then + if [ -z "$token" ] || [ -z "$organization" ] || [ -z "$cluster" ] || [ -z "$email" ]; then echo '::error::Platform local bootstrap identity is incomplete.' exit 1 fi echo "::add-mask::$token" - echo "::add-mask::$password" export RAWTREE_LIVE_API_URL=http://localhost:18087 export RAWTREE_LIVE_SESSION_TOKEN="$token" export RAWTREE_LIVE_ORGANIZATION="$organization" export RAWTREE_LIVE_CLUSTER="$cluster" export RAWTREE_LIVE_EMAIL="$email" - export RAWTREE_LIVE_PASSWORD="$password" cargo test --locked --test live_platform -- --ignored - name: Stop Platform diff --git a/Cargo.lock b/Cargo.lock index 39f509e..73cf622 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1502,7 +1502,7 @@ dependencies = [ [[package]] name = "rawtree-cli" -version = "0.7.4" +version = "0.7.5" dependencies = [ "anyhow", "axoupdater", diff --git a/Cargo.toml b/Cargo.toml index 2db37ca..0f1b67a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rawtree-cli" -version = "0.7.4" +version = "0.7.5" edition = "2021" repository = "https://github.com/rawtreedb/rawtree-cli" diff --git a/README.md b/README.md index a01fc8b..506f44e 100644 --- a/README.md +++ b/README.md @@ -543,8 +543,10 @@ To run the real test locally, start Platform from its checkout with `RAWTREE_LIVE_API_URL=http://localhost:18087` and the `RAWTREE_LIVE_SESSION_TOKEN`, `RAWTREE_LIVE_ORGANIZATION`, and `RAWTREE_LIVE_CLUSTER` values from that checkout's ignored `.env.local`. -Also set `RAWTREE_LIVE_EMAIL` and `RAWTREE_LIVE_PASSWORD` from -`RAWTREE_LOCAL_BOOTSTRAP_EMAIL` and `RAWTREE_LOCAL_BOOTSTRAP_PASSWORD` in the same file. +Also set `RAWTREE_LIVE_EMAIL` from +`RAWTREE_LOCAL_BOOTSTRAP_EMAIL` in the same file. The login test approves the +CLI device challenge with that bootstrap session, then checks saved-session use +and logout against the real API. Run `cargo test --locked --test live_platform -- --ignored` in this checkout. Run locally: diff --git a/src/cli.rs b/src/cli.rs index 047cf71..7728555 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -100,11 +100,6 @@ pub enum Command { after_help = "API key mode:\n --api-key saves an API key directly without browser authentication.\n\nAPI key output (--json):\n {\"success\":true,\"config_path\":\"\",\"database\":\"\",\"organization\":\"\",\"cluster\":\"\"}\n\nNon-interactive login:\n JSON is automatic when stdin is not a terminal. Read the device approval link\n from a JSON event on stderr while login is running. Missing selections return\n JSON choices on stdout and exit code 2. Rerun with --org, --cluster, or\n --database; each retry requires approval again." )] Login { - #[arg(long, hide = true)] - email: Option, - /// Password (prompted interactively if omitted) - #[arg(long, requires = "email", hide = true)] - password: Option, /// Do not try to open the browser automatically #[arg(long, default_value_t = false)] no_browser: bool, diff --git a/src/cli/tests.rs b/src/cli/tests.rs index ce27112..db0690f 100644 --- a/src/cli/tests.rs +++ b/src/cli/tests.rs @@ -259,34 +259,24 @@ fn login_with_token_flag_is_rejected() { } #[test] -fn login_with_password_requires_email() { - let result = Cli::try_parse_from(["rtree", "login", "--password", "secret123"]); - assert!(result.is_err(), "password without email should fail"); -} - -#[test] -fn login_with_api_key_conflicts_with_email() { - let cli = Cli::try_parse_from([ - "rtree", - "login", - "--api-key", - "rt_abc123", - "--email", - "user@example.com", - ]) - .expect("global --api-key is parsed before runtime login validation"); - assert_eq!(cli.api_key.as_deref(), Some("rt_abc123")); +fn password_login_flags_are_rejected() { + for flags in [ + vec!["--email", "user@example.com"], + vec!["--password", "secret"], + vec!["--email", "user@example.com", "--password", "secret"], + ] { + let mut args = vec!["rtree", "login"]; + args.extend(flags); + assert!(Cli::try_parse_from(args).is_err()); + } } #[test] -fn login_with_database_without_email_is_allowed_for_interactive_flow() { +fn login_with_database_is_allowed_for_interactive_flow() { let cli = Cli::try_parse_from(["rtree", "login", "--database", "analytics"]) .expect("login with --database should parse"); match cli.command { - Command::Login { - email, database, .. - } => { - assert!(email.is_none()); + Command::Login { database, .. } => { assert_eq!(database.as_deref(), Some("analytics")); } _ => panic!("expected login command"), diff --git a/src/commands/auth.rs b/src/commands/auth.rs index b230c8f..08d54e4 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -1,6 +1,6 @@ mod selection; -use selection::{resolve_auth_selection, resolve_browser_auth_selection, SelectionError}; +use selection::{resolve_browser_auth_selection, SelectionError}; use std::io::{self, Write}; use std::process::ExitCode; @@ -253,29 +253,6 @@ fn map_config_read_error(err: anyhow::Error) -> anyhow::Error { output::coded_error("config_read_failed", format!("{:#}", err), 1) } -fn update_and_save_config( - client: &ApiClient, - resp: &AuthResponse, - cli_org: Option<&str>, - cli_cluster: Option<&str>, - cli_database: Option<&str>, -) -> Result { - let mut cfg = config::load()?; - let env_org = std::env::var("RAWTREE_ORG").ok(); - let selection = resolve_auth_selection( - &client.base_url, - &resp.token, - cli_org, - cli_cluster, - cli_database, - env_org.as_deref(), - cfg.default_organization.as_deref(), - )?; - apply_auth_config(&mut cfg, &client.base_url, resp, &selection); - config::save(&cfg)?; - Ok(selection) -} - fn update_and_save_browser_config( client: &ApiClient, resp: &AuthResponse, @@ -322,48 +299,6 @@ fn clear_auth_config(cfg: &mut config::Config) { *cfg = config::Config::default(); } -pub fn login( - client: &ApiClient, - email: &str, - password: &str, - organization: Option, - cluster: Option, - database: Option, - json_mode: bool, -) -> Result<()> { - let resp: AuthResponse = client.post( - "/v1/auth/login", - &json!({"email": email, "password": password}), - )?; - - let selection = update_and_save_config( - client, - &resp, - organization.as_deref(), - cluster.as_deref(), - database.as_deref(), - )?; - let selected_organization = selection.organization.clone(); - let selected_cluster = selection.cluster.clone(); - let selected_database = selection.database.clone(); - - output::print_result( - &json!({ - "email": resp.email, - "status": "logged_in", - "selected_organization": selected_organization, - "selected_cluster": selected_cluster, - "selected_database": selected_database, - }), - json_mode, - |_| { - println!("Logged in as {}.", resp.email); - print_selected_context(&selection); - }, - ); - Ok(()) -} - pub fn login_with_api_key( client: &ApiClient, api_key: &str, diff --git a/src/commands/auth/selection.rs b/src/commands/auth/selection.rs index d025342..37bb8ec 100644 --- a/src/commands/auth/selection.rs +++ b/src/commands/auth/selection.rs @@ -120,87 +120,6 @@ struct ListClustersResponse { clusters: Vec, } -fn organization_by_name<'a>( - organizations: &'a [org::OrganizationItem], - name: &str, -) -> Option<&'a org::OrganizationItem> { - organizations.iter().find(|item| item.name == name) -} - -fn select_organization( - organizations: &[org::OrganizationItem], - cli_org: Option<&str>, - env_org: Option<&str>, - cfg_org: Option<&str>, -) -> Result> { - if let Some(name) = cli_org { - return organization_by_name(organizations, name) - .cloned() - .map(Some) - .ok_or_else(|| anyhow::anyhow!("Organization '{}' not found for current user.", name)); - } - - if let Some(name) = env_org { - if let Some(found) = organization_by_name(organizations, name) { - return Ok(Some(found.clone())); - } - } - - if let Some(name) = cfg_org { - if let Some(found) = organization_by_name(organizations, name) { - return Ok(Some(found.clone())); - } - } - - Ok(organizations.first().cloned()) -} - -fn select_database( - database_names: &[String], - selected_org: &str, - cli_database: Option<&str>, -) -> Result> { - if let Some(name) = cli_database { - return database_names - .iter() - .find(|database| database.as_str() == name) - .cloned() - .map(Some) - .ok_or_else(|| { - anyhow::anyhow!( - "Database '{}' not found in organization '{}'.", - name, - selected_org - ) - }); - } - - Ok(database_names.first().cloned()) -} - -fn select_cluster( - cluster_names: &[String], - selected_org: &str, - requested_cluster: Option<&str>, -) -> Result> { - if let Some(name) = requested_cluster { - return cluster_names - .iter() - .find(|cluster| cluster.as_str() == name) - .cloned() - .map(Some) - .ok_or_else(|| { - anyhow::anyhow!( - "Cluster '{}' not found in organization '{}'.", - name, - selected_org - ) - }); - } - - Ok(cluster_names.first().cloned()) -} - fn prompt_for_selection(label: &str, names: &[String]) -> Result> { println!("Select {}:", label); for (index, name) in names.iter().enumerate() { @@ -239,18 +158,6 @@ fn parse_selection_number(input: &str, item_count: usize) -> Option { Some(selected - 1) } -fn resolve_selected_database( - database_names_result: Result>, - selected_org: &str, - cli_database: Option<&str>, -) -> Result> { - match database_names_result { - Ok(database_names) => select_database(&database_names, selected_org, cli_database), - Err(err) if cli_database.is_some() => Err(err), - Err(_err) => Ok(None), - } -} - fn list_databases_for_organization( client: &ApiClient, organization_name: &str, @@ -358,169 +265,9 @@ pub(super) fn resolve_browser_auth_selection( }) } -pub(super) fn resolve_auth_selection( - base_url: &str, - token: &str, - cli_org: Option<&str>, - cli_cluster: Option<&str>, - cli_database: Option<&str>, - env_org: Option<&str>, - cfg_org: Option<&str>, -) -> Result { - let authed_client = ApiClient::new(base_url.to_string(), Some(token.to_string())); - let organizations = match org::list_organizations(&authed_client) { - Ok(items) => items, - Err(err) if cli_org.is_some() || cli_cluster.is_some() || cli_database.is_some() => { - return Err(err.context("failed to list organizations for auth-time selection")); - } - Err(_err) => return Ok(AuthSelection::default()), - }; - - let selected_org = select_organization(&organizations, cli_org, env_org, cfg_org)?; - let selected_org = match selected_org { - Some(item) => item, - None => { - if let Some(cluster_name) = cli_cluster { - anyhow::bail!( - "Cannot select cluster '{}' because no organization is available.", - cluster_name - ); - } - if let Some(database_name) = cli_database { - anyhow::bail!( - "Cannot select database '{}' because no organization is available.", - database_name - ); - } - return Ok(AuthSelection::default()); - } - }; - - let selected_cluster = select_cluster( - &list_clusters_for_organization(&authed_client, &selected_org.name).with_context(|| { - format!( - "failed to list clusters for organization '{}'", - selected_org.name - ) - })?, - &selected_org.name, - cli_cluster, - )?; - - let selected_database = resolve_selected_database( - list_databases_for_organization( - &authed_client, - &selected_org.name, - selected_cluster.as_deref(), - ) - .with_context(|| { - format!( - "failed to list databases for organization '{}'", - selected_org.name - ) - }), - &selected_org.name, - cli_database, - )?; - - Ok(AuthSelection { - organization: Some(selected_org.name), - cluster: selected_cluster, - database: selected_database, - }) -} - #[cfg(test)] mod tests { use super::*; - use crate::org::OrganizationItem; - - fn sample_org(name: &str) -> OrganizationItem { - OrganizationItem { - name: name.to_string(), - role: "owner".to_string(), - } - } - - #[test] - fn select_organization_uses_cli_when_present() { - let organizations = vec![sample_org("team_alpha"), sample_org("team_beta")]; - let selected = select_organization( - &organizations, - Some("team_beta"), - Some("team_alpha"), - Some("team_alpha"), - ) - .expect("selection should succeed") - .expect("organization should be selected"); - - assert_eq!(selected.name, "team_beta"); - } - - #[test] - fn select_organization_errors_for_unknown_cli_org() { - let organizations = vec![sample_org("team_alpha")]; - let result = select_organization(&organizations, Some("missing"), None, None); - assert!(result.is_err(), "unknown CLI org should fail"); - } - - #[test] - fn select_organization_uses_env_then_cfg_then_first() { - let organizations = vec![sample_org("team_alpha"), sample_org("team_beta")]; - - let env_selected = select_organization(&organizations, None, Some("team_beta"), None) - .expect("env selection should succeed") - .expect("organization should exist"); - assert_eq!(env_selected.name, "team_beta"); - - let cfg_selected = - select_organization(&organizations, None, Some("missing"), Some("team_beta")) - .expect("cfg selection should succeed") - .expect("organization should exist"); - assert_eq!(cfg_selected.name, "team_beta"); - - let first_selected = - select_organization(&organizations, None, Some("missing"), Some("also_missing")) - .expect("fallback selection should succeed") - .expect("organization should exist"); - assert_eq!(first_selected.name, "team_alpha"); - } - - #[test] - fn select_database_prefers_cli_and_fails_when_unknown() { - let databases = vec!["analytics".to_string(), "billing".to_string()]; - - let selected = select_database(&databases, "team_alpha", Some("billing")) - .expect("selection should succeed") - .expect("database should exist"); - assert_eq!(selected, "billing"); - - let err = select_database(&databases, "team_alpha", Some("missing")); - assert!(err.is_err(), "unknown CLI database should fail"); - } - - #[test] - fn select_cluster_prefers_requested_name_and_fails_when_unknown() { - let clusters = vec!["production".to_string(), "staging".to_string()]; - - let selected = select_cluster(&clusters, "team_alpha", Some("staging")) - .expect("selection should succeed") - .expect("cluster should exist"); - assert_eq!(selected, "staging"); - - let err = select_cluster(&clusters, "team_alpha", Some("missing")); - assert!(err.is_err(), "unknown cluster should fail"); - } - - #[test] - fn select_database_defaults_to_first_when_cli_missing() { - let databases = vec!["analytics".to_string(), "billing".to_string()]; - let selected = select_database(&databases, "team_alpha", None) - .expect("selection should succeed") - .expect("first database should be selected"); - assert_eq!(selected, "analytics"); - } - #[test] fn selection_number_is_one_based() { assert_eq!(parse_selection_number("1", 2), Some(0)); @@ -530,27 +277,6 @@ mod tests { assert_eq!(parse_selection_number("analytics", 2), None); } - #[test] - fn resolve_selected_database_tolerates_fetch_errors_when_cli_database_missing() { - let result = resolve_selected_database( - Err(anyhow::anyhow!("failed to list databases")), - "team_alpha", - None, - ) - .expect("implicit selection should not fail"); - assert_eq!(result, None); - } - - #[test] - fn resolve_selected_database_fails_on_fetch_errors_when_cli_database_provided() { - let result = resolve_selected_database( - Err(anyhow::anyhow!("failed to list databases")), - "team_alpha", - Some("analytics"), - ); - assert!(result.is_err(), "explicit database should remain strict"); - } - #[test] fn browser_selection_preserves_empty_and_single_choice_behavior() { for json_mode in [false, true] { diff --git a/src/main.rs b/src/main.rs index 55e4c07..4a1c122 100644 --- a/src/main.rs +++ b/src/main.rs @@ -199,26 +199,12 @@ fn effective_json_mode(cli: &Cli, stdin_is_terminal: bool) -> bool { cli.json || (matches!(cli.command, Command::Login { .. }) && !stdin_is_terminal) } -fn prompt_password_if_missing(password: Option) -> Result { - match password { - Some(p) => Ok(p), - None => { - let p = rpassword::prompt_password("Password: ")?; - if p.is_empty() { - anyhow::bail!("password cannot be empty"); - } - Ok(p) - } - } -} - fn should_prompt_for_login_method( - email: Option<&str>, json_mode: bool, stdin_is_terminal: bool, stdout_is_terminal: bool, ) -> bool { - email.is_none() && !json_mode && stdin_is_terminal && stdout_is_terminal + !json_mode && stdin_is_terminal && stdout_is_terminal } fn run(cli: Cli) -> Result { @@ -239,18 +225,11 @@ fn run(cli: Cli) -> Result { match command { Command::Login { - email, - password, no_browser, timeout_seconds, database, } => { if let Some(api_key) = cli_api_key { - if email.is_some() || password.is_some() { - anyhow::bail!( - "--api-key cannot be used with --email or --password during login" - ); - } commands::auth::login_with_api_key( &client, &api_key, @@ -260,7 +239,6 @@ fn run(cli: Cli) -> Result { json, ) } else if should_prompt_for_login_method( - email.as_deref(), json, io::stdin().is_terminal(), io::stdout().is_terminal(), @@ -289,17 +267,6 @@ fn run(cli: Cli) -> Result { ) } } - } else if let Some(email) = email { - let password = prompt_password_if_missing(password)?; - commands::auth::login( - &client, - &email, - &password, - cli_org.clone(), - login_cluster.clone(), - database, - json, - ) } else { return commands::auth::login_with_browser( &client, @@ -914,24 +881,18 @@ mod tests { #[test] fn plain_terminal_login_prompts_for_login_method() { - assert!(should_prompt_for_login_method(None, false, true, true)); + assert!(should_prompt_for_login_method(false, true, true)); } #[test] fn explicit_or_non_interactive_login_skips_login_method_prompt() { - assert!(!should_prompt_for_login_method( - Some("user@example.com"), - false, - true, - true - )); - assert!(!should_prompt_for_login_method(None, true, true, true)); - assert!(!should_prompt_for_login_method(None, false, false, true)); + assert!(!should_prompt_for_login_method(true, true, true)); + assert!(!should_prompt_for_login_method(false, false, true)); } #[test] fn redirected_stdout_skips_login_method_prompt() { - assert!(!should_prompt_for_login_method(None, false, true, false)); + assert!(!should_prompt_for_login_method(false, true, false)); } #[test] diff --git a/tests/live_platform.rs b/tests/live_platform.rs index e6124c7..2b08d75 100644 --- a/tests/live_platform.rs +++ b/tests/live_platform.rs @@ -4,7 +4,8 @@ mod storage; mod workflows; use std::env; -use std::process::{Command, Output}; +use std::io::{BufRead, BufReader}; +use std::process::{Command, Output, Stdio}; use std::time::{SystemTime, UNIX_EPOCH}; use serde_json::{json, Value}; @@ -30,7 +31,7 @@ impl LiveCli { } } - fn execute(&self, args: &[&str], token: Option<&str>) -> Output { + fn command(&self, args: &[&str], token: Option<&str>) -> Command { let mut command = Command::new(env!("CARGO_BIN_EXE_rtree")); for (name, _) in env::vars().filter(|(name, _)| name.starts_with("RAWTREE_")) { command.env_remove(name); @@ -45,7 +46,11 @@ impl LiveCli { if let Some(token) = token { command.env("RAWTREE_API_KEY", token); } - command.output().expect("run rtree") + command + } + + fn execute(&self, args: &[&str], token: Option<&str>) -> Output { + self.command(args, token).output().expect("run rtree") } fn json(&self, args: &[&str]) -> Value { @@ -68,23 +73,45 @@ impl LiveCli { fn logout_removes_saved_authentication_against_real_platform() { let cli = LiveCli::from_environment(); let email = env::var("RAWTREE_LIVE_EMAIL").expect("RAWTREE_LIVE_EMAIL is required"); - let password = env::var("RAWTREE_LIVE_PASSWORD").expect("RAWTREE_LIVE_PASSWORD is required"); let config_path = cli.home.path().join(".config/rtree/config.json"); assert!(!config_path.exists()); - let login = cli.json_with_token( - &[ - "--org", - &cli.organization, - "--cluster", - &cli.cluster, - "login", - "--email", - &email, - "--password", - &password, - ], - None, - ); + let mut child = cli + .command( + &[ + "--org", + &cli.organization, + "--cluster", + &cli.cluster, + "login", + "--no-browser", + "--database", + "default", + "--timeout-seconds", + "60", + ], + None, + ) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("start browser login"); + let mut approval_event = String::new(); + BufReader::new(child.stderr.take().expect("login events")) + .read_line(&mut approval_event) + .expect("read device approval event"); + let event: Value = serde_json::from_str(&approval_event).expect("device approval JSON"); + assert_eq!(event["event"], "device_approval_required"); + reqwest::blocking::Client::new() + .post(format!("{}/v1/auth/cli/device/approve", cli.url)) + .bearer_auth(&cli.token) + .json(&json!({"user_code": event["user_code"]})) + .send() + .expect("approve device") + .error_for_status() + .expect("device approved"); + let output = child.wait_with_output().expect("finish browser login"); + assert!(output.status.success(), "browser login failed"); + let login: Value = serde_json::from_slice(&output.stdout).expect("login JSON"); assert_eq!(login["status"], "logged_in"); assert_eq!(login["email"], email);