diff --git a/docs/guides/login-and-profiles.md b/docs/guides/login-and-profiles.md index fcd3c9f..40f1851 100644 --- a/docs/guides/login-and-profiles.md +++ b/docs/guides/login-and-profiles.md @@ -69,7 +69,7 @@ voltage profiles delete staging ## See the effective scope -`voltage context` shows the effective profile, account, organization, environments, wallet, API URL, and configuration directory, and where each came from (`flag`, `profile`, `environment`, `default`, or `unset`). It reports why commands would reject the credential selection as `credential_problem` (`empty_api_key`, `no_credential`, `multiple_credentials`, `unknown_credential`, or `bound_elsewhere` for an API key used outside its organization or environment), using the same selection commands use. It also lists `VOLTAGE_*` variables that are set but ignored because a flag or the selected profile takes precedence. It reads no saved secret, makes no network request, and creates no files: +`voltage context` shows the effective profile, account, organization, environments, wallet, API and auth URLs, and configuration directory, and where each came from (`flag`, `profile`, `environment`, `default`, or `unset`). It reports why commands would reject the credential selection as `credential_problem` (`empty_api_key`, `no_credential`, `multiple_credentials`, `unknown_credential`, `bound_elsewhere` for an API key used outside its organization or environment, or `auth_url_mismatch` for a browser login used with a different `--auth-url`), using the same selection commands use. It also lists `VOLTAGE_*` variables that are set but ignored because a flag or the selected profile takes precedence. It reads no saved secret, makes no network request, and creates no files: ```sh voltage context --profile staging diff --git a/docs/reference/exit-codes.md b/docs/reference/exit-codes.md index 1414ec2..b6e37cb 100644 --- a/docs/reference/exit-codes.md +++ b/docs/reference/exit-codes.md @@ -13,4 +13,4 @@ After exit code 4 for a payment, check its ID before you retry; see [payment IDs](../concepts/safety-model.md#payment-ids-and-the-request-journal). After exit code 5, keep watching with `voltage payments get PAYMENT_ID --profile prod --wait completed`. -Many errors for exit codes 2 and 3 carry a `hint:` (or `error.hint` with `--json`) that names the next command: the discovery command for a missing `--org`, `--env`, or `--wallet`, the saved credential and profile names for an unknown or ambiguous selection, `voltage logout --local --account NAME` and then `voltage login --account NAME` for an expired or revoked login, `voltage auth status` for a rejected account credential (HTTP 401), and `voltage context` for an account credential that cannot reach the selected scope (HTTP 403). A rejected checkout token points to `--token-file` or the variable that supplies it, never to the account credential. A payment rejected with `quote_required` points to `voltage quotes create`. +Many errors for exit codes 2 and 3 carry a `hint:` (or `error.hint` with `--json`) that names the next command: the discovery command for a missing `--org`, `--env`, or `--wallet`, the saved credential and profile names for an unknown or ambiguous selection, `voltage logout --local --account NAME` and then `voltage login --account NAME` for an expired or revoked login, `voltage logout --account NAME` and then `voltage login --account NAME --auth-url URL` to move a browser login to a different auth URL, `voltage auth status` for a rejected account credential (HTTP 401), and `voltage context` for an account credential that cannot reach the selected scope (HTTP 403). A rejected checkout token points to `--token-file` or the variable that supplies it, never to the account credential. A payment rejected with `quote_required` points to `voltage quotes create`. diff --git a/docs/reference/output.md b/docs/reference/output.md index 6f520ee..d740433 100644 --- a/docs/reference/output.md +++ b/docs/reference/output.md @@ -25,7 +25,7 @@ API fields stay inside `data`. With `--all`, JSON output collects the pages into ## Streams -Diagnostics, prompts, and progress go to stderr. Interactive stderr shows immediate, cancellable status for network requests and waits; redirected stderr gets no spinner/control codes. `-q, --quiet` suppresses optional status and notices, never requested results, errors, warnings, or recovery IDs. +Diagnostics, prompts, and progress go to stderr. Interactive stderr shows immediate, cancellable status for network requests and waits, with the time waited so far on payment waits and device approval; redirected stderr gets no spinner/control codes. `-q, --quiet` suppresses optional status and notices, never requested results, errors, warnings, or recovery IDs. Human errors start with `error:`, which leads with the API's reason when it rejects a request with a client error (HTTP 4xx), and may include a safe `hint:` and an indented `details:` block of `field value` lines. `--json` reports the complete API response in `error.detail`, and makes command-line parse errors machine-readable as well as runtime errors. A downstream consumer that closes stdout early is treated as successful pipeline completion. diff --git a/src/api.rs b/src/api.rs index 9bf3d14..c181cc0 100644 --- a/src/api.rs +++ b/src/api.rs @@ -21,7 +21,7 @@ use crate::{ payment::{PaymentDirection, PaymentView, ReceiveKind, StatusText, WaitProgress, WaitTarget}, registry::{AuthScheme, Method, Operation, OperationId, operation}, secret::Secret, - terminal::Terminal, + terminal::{self, Terminal}, }; use futures_util::StreamExt; use serde::{Deserialize, Deserializer, Serialize}; @@ -1124,6 +1124,7 @@ async fn wait_for_payment( let mut invoice_presented = false; let mut last_status: Option = None; let mut last_notice = Instant::now() - STATUS_NOTICE_INTERVAL; + let started = Instant::now(); loop { let view = PaymentView::from_body(&response.body); if let Some(status) = &view.status @@ -1177,7 +1178,15 @@ async fn wait_for_payment( )) .with_detail(ErrorDetail::pending(id))); } - tokio::time::sleep(pause).await; + api.terminal + .during( + format!( + "Waiting for the payment to be {target}... {}", + terminal::elapsed(started.elapsed()) + ), + tokio::time::sleep(pause), + ) + .await; match tokio::time::timeout_at(deadline, api.read(&path)).await { Ok(Ok(next)) => { pause = next.retry_after.unwrap_or_else(|| backoff.pause()); diff --git a/src/auth.rs b/src/auth.rs index 0162150..b045e8b 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -13,7 +13,7 @@ use crate::{ Login, Scope, Settings, read_secret, }, secret::Secret, - terminal::Terminal, + terminal::{self, Terminal}, }; use serde::{Deserialize, Serialize, de::DeserializeOwned}; use serde_json::Value; @@ -402,6 +402,20 @@ pub(crate) fn select<'a>( selected: Some(Selected::Saved { name, account }), }); } + // A login refreshes only at its recorded origin. An invalid flag is not a mismatch here: + // `resolve` reports it as an invalid URL. + if account.kind == AccountKind::User + && flags + .auth_url + .as_deref() + .and_then(|raw| base_url(raw).ok()) + .is_some_and(|url| url != account.auth_url) + { + return Err(Rejected { + problem: CredentialProblem::AuthUrlMismatch, + selected: Some(Selected::Saved { name, account }), + }); + } Ok(Selected::Saved { name, account }) } @@ -492,13 +506,17 @@ async fn poll_device_grant( .interval .unwrap_or(MIN_POLL_INTERVAL) .clamp(MIN_POLL_INTERVAL, MAX_POLL_INTERVAL); + let started = tokio::time::Instant::now(); loop { if tokio::time::Instant::now() + Duration::from_secs(interval) >= deadline { return Err(Error::timeout("Login expired; run voltage login again")); } terminal .during( - "Waiting for device approval...", + format!( + "Waiting for device approval... {}", + terminal::elapsed(started.elapsed()) + ), tokio::time::sleep(Duration::from_secs(interval)), ) .await; @@ -634,12 +652,8 @@ pub async fn resolve( match account.kind { AccountKind::ApiKey => settings.read_credential(&name), AccountKind::User => { + // `select` rejected a valid flag that differs from the saved URL. let url = base_url(flags.auth_url.as_deref().unwrap_or(&account.auth_url))?; - if url != account.auth_url { - return Err(Error::usage( - "Saved credentials cannot be refreshed at a different auth URL", - )); - } // Credential writes replace the whole value atomically, so a fresh login is used // without the lock. Only a refresh, which spends the single-use refresh token, is // serialized. diff --git a/src/config.rs b/src/config.rs index 63c5469..9e090af 100644 --- a/src/config.rs +++ b/src/config.rs @@ -107,6 +107,9 @@ pub(crate) enum CredentialProblem { UnknownCredential, /// The saved API key is bound to a different organization or environment. BoundElsewhere, + /// `--auth-url` names a different auth service than the one the browser login was saved + /// with, and a login refreshes only at its recorded origin. + AuthUrlMismatch, } impl CredentialProblem { @@ -139,6 +142,12 @@ impl CredentialProblem { "Saved credentials: {}. Select a key for this scope or a browser login with --account NAME.", saved.unwrap_or_default() )), + Self::AuthUrlMismatch => Error::usage( + "Saved credentials cannot be refreshed at a different auth URL", + ) + .with_hint( + "Omit --auth-url to use the URL the login was saved with (`voltage context` shows it). To move the login to this URL, run `voltage logout --account NAME`, then `voltage login --account NAME --auth-url URL`.", + ), } } } diff --git a/src/context.rs b/src/context.rs index fed5628..ff95348 100644 --- a/src/context.rs +++ b/src/context.rs @@ -8,7 +8,9 @@ use crate::{ Result, auth::{self, Rejected, Selected}, cli::GlobalFlags, - config::{self, API_URL, AccountKind, CredentialProblem, CredentialStore, Settings, Source}, + config::{ + self, API_URL, AUTH_URL, AccountKind, CredentialProblem, CredentialStore, Settings, Source, + }, }; use serde::Serialize; use uuid::Uuid; @@ -81,6 +83,11 @@ pub struct Context { wallet_id: Setting>, webhook_id: Setting>, api_url: Setting, + /// Where a browser login signs in and refreshes. A selected browser login always uses the + /// URL it was saved with (sourced like the credential); a different `--auth-url` is the + /// `auth_url_mismatch` credential problem. An API key never uses the auth service, so + /// otherwise it is the normalized flag, else the built-in default. + auth_url: Setting, config_dir: Setting, config_file: ConfigFile, /// Whether changes are sent without `--execute`, from `VOLTAGE_EXECUTE`, so an exported @@ -123,6 +130,23 @@ pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { }, }; let credential = selected.as_ref().map(CredentialSummary::from); + let flag = global.auth_url.as_deref().map(auth::base_url).transpose()?; + let auth_url = match (&selected, flag) { + (Some(Selected::Saved { account: saved, .. }), _) if saved.kind == AccountKind::User => { + Setting { + value: saved.auth_url.clone(), + source: account.source, + } + } + (_, Some(url)) => Setting { + value: url, + source: Source::Flag, + }, + (_, None) => Setting { + value: AUTH_URL.to_owned(), + source: Source::Default, + }, + }; let config_file = settings.config_file(); Ok(Context { profile: Setting { @@ -162,6 +186,7 @@ pub fn inspect(settings: &Settings, global: &GlobalFlags) -> Result { source: Source::Default, }, }, + auth_url, config_dir: Setting { value: settings.dir.display().to_string(), source: config_dir_source, diff --git a/src/output/human.rs b/src/output/human.rs index 5484849..88e3e9a 100644 --- a/src/output/human.rs +++ b/src/output/human.rs @@ -276,16 +276,16 @@ fn amount(value: &Value) -> Option { Some(scrub_line(&format!("{sign}{amount} {unit}"))) } -/// One `field value` line, with where a local setting's value came from. +/// One field: its value's lines, and where a local setting's value came from. struct Line { key: String, - value: String, + values: Vec, source: Option, } -/// Nested objects flatten to dotted field names; amounts stay whole. In a local result, a -/// `{value, source}` setting is one line, `name value (source)`, and string lists are -/// comma-separated. +/// Nested objects flatten to dotted field names; amounts stay whole. A list of strings, such +/// as a dry run's notes, puts each item on its own line under the first. In a local result, a +/// `{value, source}` setting reads `name value (source)`. fn fields(out: &mut impl Write, map: &Map, local: bool) -> Result<()> { let mut lines = Vec::new(); flatten(String::new(), map, local, &mut lines); @@ -294,10 +294,22 @@ fn fields(out: &mut impl Write, map: &Map, local: bool) -> Result .map(|line| line.key.chars().count()) .max() .unwrap_or(0); - for Line { key, value, source } in lines { - match source { - Some(source) => writeln!(out, "{key: writeln!(out, "{key:, local: bool, lines: &mut Ve if local && let Some((value, source)) = value.as_object().and_then(setting) { lines.push(Line { key, - value: local_cell(value), + values: value_lines(value), source: Some(scrub_line(source)), }); continue; @@ -324,11 +336,7 @@ fn flatten(prefix: String, map: &Map, local: bool, lines: &mut Ve } other => lines.push(Line { key, - value: if local { - local_cell(other) - } else { - cell(other) - }, + values: value_lines(other), source: None, }), } @@ -343,15 +351,16 @@ fn setting(map: &Map) -> Option<(&Value, &str)> { Some((map.get("value")?, map.get("source")?.as_str()?)) } -/// Like `cell`, but a list of strings is comma-separated, or `-` when empty. -fn local_cell(value: &Value) -> String { +/// A field's value as display lines: one per item of a list of strings, `-` for an empty +/// list, and otherwise the single `cell`. +fn value_lines(value: &Value) -> Vec { let strings = value .as_array() .and_then(|items| items.iter().map(Value::as_str).collect::>>()); match strings { - Some(items) if items.is_empty() => EMPTY_CELL.into(), - Some(items) => scrub_line(&items.join(", ")), - None => cell(value), + Some(items) if items.is_empty() => vec![EMPTY_CELL.into()], + Some(items) => items.into_iter().map(scrub_line).collect(), + None => vec![cell(value)], } } @@ -495,16 +504,19 @@ mod tests { #[test] fn single_resources_render_as_flattened_fields() { let text = rendered( - json!({"id": "w", "balance": {"available": {"amount": 5, "currency": "usd"}, "held": null}, "tags": ["a", "b"], "empty": {}}), + json!({"id": "w", "balance": {"available": {"amount": 5, "currency": "usd"}, "held": null}, "tags": ["a", "b"], "none": [], "empty": {}}), None, ); + // Each list item gets its own line; an empty list reads `-`, not `[]`. assert_eq!( text, "balance.available 5 cents\n\ balance.held -\n\ empty {}\n\ id w\n\ - tags [\"a\",\"b\"]\n" + none -\n\ + tags a\n\ + \x20 b\n" ); } @@ -528,7 +540,8 @@ mod tests { "config_file.exists false\n\ config_file.path /c/config.toml\n\ credential -\n\ - environment_ids {ID}, e2 (profile)\n\ + environment_ids {ID} (profile)\n\ + \x20 e2\n\ ignored_variables VOLTAGE_WALLET_ID\n\ notes -\n\ organization_id {ID} (flag)\n\ diff --git a/src/terminal.rs b/src/terminal.rs index 2ac22a0..0cbe154 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -6,6 +6,7 @@ use inquire::{InquireError, Select}; use std::{ fmt::{self, Display}, io::{ErrorKind, IsTerminal, Write}, + time::Duration, }; use tokio::sync::oneshot; use uuid::Uuid; @@ -30,7 +31,7 @@ impl Terminal { /// Optional status for a human, cleared when the guard drops; no control codes on /// redirected stderr. - pub fn progress(self, label: &'static str) -> Progress { + pub fn progress(self, label: impl Display) -> Progress { let active = !self.quiet && std::io::stderr().is_terminal(); if active { eprint!("{label}"); @@ -41,7 +42,7 @@ impl Terminal { /// Show `label` while `work` runs. The status cannot outlive the work: it clears on /// completion, error, or cancellation. - pub async fn during(self, label: &'static str, work: impl Future) -> T { + pub async fn during(self, label: impl Display, work: impl Future) -> T { let _progress = self.progress(label); work.await } @@ -255,6 +256,17 @@ impl Drop for TerminalModes { } } +/// How long a wait has run, for a status line that would otherwise look stuck: `42s`, or +/// `3m 05s` from a minute on. +pub fn elapsed(duration: Duration) -> String { + let seconds = duration.as_secs(); + if seconds < 60 { + format!("{seconds}s") + } else { + format!("{}m {:02}s", seconds / 60, seconds % 60) + } +} + /// Clears its status line when dropped, including when Ctrl-C cancels the owning future. #[must_use = "progress is cleared as soon as the guard is dropped"] pub struct Progress { @@ -296,6 +308,14 @@ mod tests { } } + #[test] + fn elapsed_time_reads_in_seconds_then_minutes() { + assert_eq!(elapsed(Duration::from_millis(900)), "0s"); + assert_eq!(elapsed(Duration::from_secs(59)), "59s"); + assert_eq!(elapsed(Duration::from_secs(60)), "1m 00s"); + assert_eq!(elapsed(Duration::from_secs(185)), "3m 05s"); + } + #[test] fn scrubbing_removes_controls_and_bidirectional_marks() { let hostile = "a\u{1b}[2J\u{9b}b\u{202e}c\u{200b}d\te\nf"; diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 5563daf..fe21dbf 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -913,6 +913,11 @@ async fn context_reports_effective_scope_and_sources_without_reading_secrets() { assert_eq!(data["wallet_id"], setting(json!(WALLET), "flag")); assert_eq!(data["webhook_id"], setting(Value::Null, "unset")); assert_eq!(data["api_url"], setting(json!(server.uri()), "flag")); + // A saved key never uses the auth service, so its URL is the default, not the profile's. + assert_eq!( + data["auth_url"], + setting(json!(config::AUTH_URL), "default") + ); assert_eq!( data["config_dir"], setting(json!(dir.path().display().to_string()), "flag") @@ -1012,6 +1017,89 @@ async fn context_reports_effective_scope_and_sources_without_reading_secrets() { assert!(server.received_requests().await.unwrap().is_empty()); } +#[tokio::test] +async fn context_reports_the_auth_url_commands_would_use() { + let (server, dir) = fixture().await; + let auth = MockServer::start().await; + seeded_login(dir.path(), &auth, false); + let setting = |value: &str, source: &str| json!({"value": value, "source": source}); + let context = |args: &[&str], ambient: bool| { + let mut command = cli(dir.path(), &server); + if !ambient { + command.env_remove("VOLTAGE_API_KEY"); + } + json_stdout(&command.arg("context").args(args).assert().success())["data"].clone() + }; + + // The ambient key uses no auth service: the built-in default, or the normalized flag. + let data = context(&[], true); + assert_eq!(data["auth_url"], setting(config::AUTH_URL, "default")); + let data = context(&["--auth-url", "https://auth.example.com/api/v1/"], true); + assert_eq!( + data["auth_url"], + setting("https://auth.example.com/api/v1", "flag") + ); + assert_eq!(data["credential_problem"], Value::Null); + + // A browser login reports the URL it was saved with, sourced like the credential. A flag + // that normalizes to it is no problem. + let saved = auth.uri(); + let data = context(&[], false); + assert_eq!(data["auth_url"], setting(&saved, "default")); + let data = context( + &["--account", LOGIN_NAME, "--auth-url", &format!("{saved}/")], + false, + ); + assert_eq!(data["auth_url"], setting(&saved, "flag")); + assert_eq!(data["credential_problem"], Value::Null); + + // A different flag is reported the way commands reject it. + let other = ["--auth-url", "https://auth.example.com/api/v1"]; + let data = context(&other, false); + assert_eq!(data["auth_url"], setting(&saved, "default")); + assert_eq!( + data["credential"], + json!({"kind": "user", "location": "file"}) + ); + assert_eq!(data["credential_problem"], "auth_url_mismatch"); + let rejected = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(["wallets", "list", "--org", ORG, "--env", ENV]) + .args(other) + .assert() + .code(2); + let shown = stderr(&rejected); + assert!( + shown.contains("Saved credentials cannot be refreshed at a different auth URL"), + "{shown}" + ); + assert!(shown.contains("Omit --auth-url"), "{shown}"); + // `login` refuses a saved name, so moving the login names a revoking logout first. + assert!( + shown.contains( + "run `voltage logout --account NAME`, then `voltage login --account NAME --auth-url URL`" + ), + "{shown}" + ); + + // An invalid flag is an invalid URL, for context and commands alike. + let invalid = ["--auth-url", "http://auth.example.com"]; + for command in [ + &["context"][..], + &["wallets", "list", "--org", ORG, "--env", ENV], + ] { + let failed = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(command) + .args(invalid) + .assert() + .code(2); + assert!(stderr(&failed).contains("Service URLs require HTTPS")); + } + assert!(server.received_requests().await.unwrap().is_empty()); + assert!(auth.received_requests().await.unwrap().is_empty()); +} + #[tokio::test] async fn context_inspection_creates_no_configuration() { let (server, dir) = fixture().await; @@ -4017,6 +4105,94 @@ async fn a_signal_at_the_picker_restores_the_terminal() { assert_eq!(run.modes.0, run.modes.1); } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_payment_wait_on_a_terminal_shows_how_long_it_has_waited() { + let (server, dir) = fixture().await; + route("GET", payment_path()) + .respond_with(in_order(vec![ + ok(json!({"id": RESOURCE, "status": "sending"})), + ok(json!({"id": RESOURCE, "status": "completed"})), + ])) + .expect(2) + .mount(&server) + .await; + let run = on_a_terminal( + dir.path(), + &server, + &[ + "payments", + "get", + RESOURCE, + "--org", + ORG, + "--env", + ENV, + "--wait", + "completed", + ], + &[], + ) + .await; + assert_eq!(run.code, Some(0), "{}", run.shown); + assert!( + run.shown + .contains("Waiting for the payment to be completed... 0s"), + "{}", + run.shown + ); + server.verify().await; +} + +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn device_approval_on_a_terminal_shows_how_long_it_has_waited() { + let (server, dir) = fixture().await; + respond_once( + &server, + route("POST", "/oauth/device_authorization"), + ok(json!({ + "device_code":"device-secret","user_code":"ABCD-EFGH", + "verification_uri":"https://app.voltage.cloud/cli/authorize","expires_in":60,"interval":5 + })), + ) + .await; + respond_once( + &server, + route("POST", "/oauth/token"), + ok(json!({ + "access_token":"login-access-secret","refresh_token":"login-refresh-secret", + "token_type":"Bearer","expires_in":600 + })), + ) + .await; + respond_once( + &server, + route("GET", "/users/current"), + ok(json!({"id":RESOURCE,"email":LOGIN_EMAIL})), + ) + .await; + let uri = server.uri(); + let args = [ + "login", + "--auth-url", + &uri, + "--no-browser", + "--account", + LOGIN_NAME, + "--credential-store", + "file", + ]; + let run = on_a_terminal(dir.path(), &server, &args, &[]).await; + assert_eq!(run.code, Some(0), "{}", run.shown); + assert!( + run.shown.contains("Waiting for device approval... 0s"), + "{}", + run.shown + ); + server.verify().await; +} + // --------------------------------------------------------------------------------------- // Prices and conversions // ---------------------------------------------------------------------------------------