diff --git a/README.md b/README.md index bb6a23c..ec456c3 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,10 @@ new binary becomes the creator of fresh Keychain entries. **Click "Always Allow"** on the two prompts macOS shows during re-store — otherwise future commands will prompt again. +For CI or headless boxes, pass credentials via env vars or flags +(e.g. `JR_EMAIL="..." JR_API_TOKEN="$TOKEN" jr --no-input auth refresh`) +so the flow completes without a TTY. + Tracked in [#207](https://github.com/Zious11/jira-cli/issues/207). A longer-term fix (Developer ID signing) is tracked as a separate issue. @@ -86,6 +90,12 @@ jr auth login # Or authenticate with OAuth 2.0 (requires your own OAuth app) jr auth login --oauth +# Non-interactive (CI / agents): flags or env vars, no TTY required. +# Prefer env vars for secrets — bare CLI args can leak via process lists. +JR_EMAIL="you@example.com" JR_API_TOKEN="$TOKEN" jr --no-input auth login +JR_OAUTH_CLIENT_ID="$ID" JR_OAUTH_CLIENT_SECRET="$SECRET" \ + jr --no-input auth login --oauth + # View your current sprint/board issues jr issue list --project FOO @@ -134,7 +144,8 @@ jr issue comment KEY-123 "Deployed to staging" | Command | Description | |---------|-------------| | `jr init` | Configure Jira instance and authenticate | -| `jr auth login` | Authenticate with API token (default) or `--oauth` for OAuth 2.0 | +| `jr auth login` | Authenticate with API token (default) or `--oauth` for OAuth 2.0. Non-interactive: `--email`/`--token` or `JR_EMAIL`/`JR_API_TOKEN`; `--client-id`/`--client-secret` or `JR_OAUTH_CLIENT_ID`/`JR_OAUTH_CLIENT_SECRET` for OAuth | +| `jr auth refresh` | Clear stored credentials and re-run login (same flags/env vars as `auth login`) | | `jr auth status` | Show authentication status | | `jr me` | Show current user info | | `jr issue list` | List issues (`--assignee`, `--reporter`, `--recent`, `--status`, `--open`, `--team`, `--asset KEY`, `--jql`, `--limit`/`--all`, `--points`, `--assets`) | @@ -227,6 +238,7 @@ board_id = 42 - `--url-only` prints URLs instead of opening a browser - State-changing commands are idempotent (exit 0 if already in target state) - Structured exit codes (see [Exit Codes](#exit-codes) table) +- `auth login` / `auth refresh` accept credentials via flags (`--email`, `--token`, `--client-id`, `--client-secret`) or env vars (`JR_EMAIL`, `JR_API_TOKEN`, `JR_OAUTH_CLIENT_ID`, `JR_OAUTH_CLIENT_SECRET`) — no TTY required. Prefer env vars for secrets. ```bash # AI agent workflow example diff --git a/src/cli/auth.rs b/src/cli/auth.rs index aa60e5c..7fb2175 100644 --- a/src/cli/auth.rs +++ b/src/cli/auth.rs @@ -3,8 +3,77 @@ use dialoguer::{Input, Password}; use crate::api::auth; use crate::config::Config; +use crate::error::JrError; use crate::output; +/// Environment variable names for the four auth credentials. +/// +/// Flag > env > prompt precedence is implemented by [`resolve_credential`]. +/// Callers pass the matching `flag_name` so error messages can cite both +/// names verbatim. +pub(crate) const ENV_EMAIL: &str = "JR_EMAIL"; +pub(crate) const ENV_API_TOKEN: &str = "JR_API_TOKEN"; +pub(crate) const ENV_OAUTH_CLIENT_ID: &str = "JR_OAUTH_CLIENT_ID"; +pub(crate) const ENV_OAUTH_CLIENT_SECRET: &str = "JR_OAUTH_CLIENT_SECRET"; + +/// Resolve a credential value via flag → env → TTY prompt, or error under +/// `--no-input`. +/// +/// Order of precedence: +/// 1. `flag_value` (explicit CLI arg wins). +/// 2. `env::var(env_name)` if non-empty. +/// 3. If `no_input` is true, return a `JrError::UserError` naming the flag +/// and env var so scripts/agents can recover. `hint` — if supplied — +/// is appended to the error so first-time agents learn *where to obtain* +/// the credential, not just how to pass it (relevant for OAuth where +/// users must first create an app at developer.atlassian.com). +/// 4. Otherwise, prompt interactively. `is_password` chooses between +/// `dialoguer::Password` (masked) and `Input` (visible). +/// +/// Empty env values are ignored so an accidentally-exported-but-unset var +/// doesn't silently substitute for real input. +pub(crate) fn resolve_credential( + flag_value: Option, + env_name: &str, + flag_name: &str, + prompt_label: &str, + is_password: bool, + no_input: bool, + hint: Option<&str>, +) -> Result { + if let Some(v) = flag_value.filter(|v| !v.is_empty()) { + return Ok(v); + } + if let Ok(v) = std::env::var(env_name) + && !v.is_empty() + { + return Ok(v); + } + if no_input { + let base = format!("{prompt_label} is required. Provide {flag_name} or set ${env_name}."); + let msg = match hint { + Some(h) => format!("{base} {h}"), + None => base, + }; + return Err(JrError::UserError(msg).into()); + } + if is_password { + Password::new() + .with_prompt(prompt_label) + .interact() + .with_context(|| format!("failed to read {prompt_label}")) + } else { + Input::new() + .with_prompt(prompt_label) + .interact_text() + .with_context(|| format!("failed to read {prompt_label}")) + } +} + +/// Hint for OAuth client_id / client_secret errors so first-time agents +/// discover they must create an OAuth app before passing credentials. +const OAUTH_APP_HINT: &str = "Create one at https://developer.atlassian.com/console/myapps/."; + /// Which auth flow `jr auth refresh` should dispatch to. /// /// Internal detail of the `refresh` command; kept module-private so it @@ -44,17 +113,30 @@ fn chosen_flow(config: &Config, oauth_override: bool) -> AuthFlow { } } -/// Prompt for email and API token, then store in keychain. -pub async fn login_token() -> Result<()> { - let email: String = dialoguer::Input::new() - .with_prompt("Jira email") - .interact_text() - .context("failed to read Jira email")?; - - let token: String = dialoguer::Password::new() - .with_prompt("API token") - .interact() - .context("failed to read API token")?; +/// Resolve email and API token (flag → env → prompt), then store in keychain. +pub async fn login_token( + email: Option, + token: Option, + no_input: bool, +) -> Result<()> { + let email = resolve_credential( + email, + ENV_EMAIL, + "--email", + "Jira email", + false, + no_input, + None, + )?; + let token = resolve_credential( + token, + ENV_API_TOKEN, + "--token", + "API token", + true, + no_input, + None, + )?; auth::store_api_token(&email, &token)?; eprintln!("Credentials stored in keychain."); @@ -62,20 +144,37 @@ pub async fn login_token() -> Result<()> { } /// Run the OAuth 2.0 (3LO) login flow and persist site configuration. -/// Prompts the user for their own OAuth app credentials. -pub async fn login_oauth() -> Result<()> { - eprintln!("OAuth 2.0 requires your own Atlassian OAuth app."); - eprintln!("Create one at: https://developer.atlassian.com/console/myapps/\n"); - - let client_id: String = Input::new() - .with_prompt("OAuth Client ID") - .interact_text() - .context("failed to read OAuth client ID")?; +/// +/// Credentials resolved via flag → env → prompt, so CI/agent workflows can +/// pipe them in without a TTY. +pub async fn login_oauth( + client_id: Option, + client_secret: Option, + no_input: bool, +) -> Result<()> { + if !no_input { + eprintln!("OAuth 2.0 requires your own Atlassian OAuth app."); + eprintln!("Create one at: https://developer.atlassian.com/console/myapps/\n"); + } - let client_secret: String = Password::new() - .with_prompt("OAuth Client Secret") - .interact() - .context("failed to read OAuth client secret")?; + let client_id = resolve_credential( + client_id, + ENV_OAUTH_CLIENT_ID, + "--client-id", + "OAuth Client ID", + false, + no_input, + Some(OAUTH_APP_HINT), + )?; + let client_secret = resolve_credential( + client_secret, + ENV_OAUTH_CLIENT_SECRET, + "--client-secret", + "OAuth Client Secret", + true, + no_input, + Some(OAUTH_APP_HINT), + )?; // Store OAuth app credentials in keychain crate::api::auth::store_oauth_app_credentials(&client_id, &client_secret)?; @@ -152,6 +251,11 @@ fn refresh_success_payload(flow: AuthFlow) -> serde_json::Value { /// before the error is propagated. pub async fn refresh_credentials( oauth_override: bool, + email: Option, + token: Option, + client_id: Option, + client_secret: Option, + no_input: bool, output: &crate::cli::OutputFormat, ) -> Result<()> { let config = Config::load()?; @@ -162,8 +266,8 @@ pub async fn refresh_credentials( )?; let login_result = match flow { - AuthFlow::Token => login_token().await, - AuthFlow::OAuth => login_oauth().await, + AuthFlow::Token => login_token(email, token, no_input).await, + AuthFlow::OAuth => login_oauth(client_id, client_secret, no_input).await, }; if let Err(err) = login_result { @@ -263,4 +367,145 @@ mod tests { assert_eq!(payload["status"], "refreshed"); assert_eq!(payload["auth_method"], "oauth"); } + + // ── resolve_credential ─────────────────────────────────────────── + // + // Env-reading tests must serialize process-environment mutation across + // parallel test threads. `std::env::set_var` / `remove_var` are unsafe + // in Rust 2024 because concurrent env access (even on different keys) + // is UB — C's getenv/setenv aren't thread-safe. `EnvGuard` holds + // `ENV_LOCK` for its full lifetime and removes the var on drop so a + // panic mid-test doesn't leak state to later tests in the same + // process. Matches the pattern in src/config.rs::ENV_MUTEX. + + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + struct EnvGuard { + key: &'static str, + _lock: std::sync::MutexGuard<'static, ()>, + } + + impl EnvGuard { + fn set(key: &'static str, value: &str) -> Self { + let lock = ENV_LOCK.lock().unwrap(); + // SAFETY: test env mutation is serialized by ENV_LOCK, held for + // this guard's lifetime. The Drop impl unsets the same + // test-local key before releasing the lock. + unsafe { + std::env::set_var(key, value); + } + EnvGuard { key, _lock: lock } + } + } + + impl Drop for EnvGuard { + fn drop(&mut self) { + // SAFETY: matches the test-local key set in `EnvGuard::set` + // while `_lock` is still held by this `EnvGuard`. + unsafe { + std::env::remove_var(self.key); + } + } + } + + #[test] + fn resolve_credential_prefers_flag_over_env() { + let _guard = EnvGuard::set("_JR_TEST_PREFERS_FLAG", "from-env"); + let got = resolve_credential( + Some("from-flag".into()), + "_JR_TEST_PREFERS_FLAG", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap(); + assert_eq!(got, "from-flag"); + } + + #[test] + fn resolve_credential_falls_back_to_env_when_flag_absent() { + let _guard = EnvGuard::set("_JR_TEST_FALLS_BACK", "from-env"); + let got = resolve_credential( + None, + "_JR_TEST_FALLS_BACK", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap(); + assert_eq!(got, "from-env"); + } + + #[test] + fn resolve_credential_ignores_empty_flag_and_env() { + // Empty values should fall through to the no_input error path. + let _guard = EnvGuard::set("_JR_TEST_EMPTY", ""); + let err = resolve_credential( + Some(String::new()), + "_JR_TEST_EMPTY", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap_err(); + assert!( + err.downcast_ref::() + .is_some_and(|e| matches!(e, JrError::UserError(_))), + "Expected JrError::UserError for empty inputs, got: {err}" + ); + } + + #[test] + fn resolve_credential_no_input_errors_when_missing() { + // resolve_credential reads env via std::env::var — hold ENV_LOCK to + // serialize against set/remove calls in sibling tests. + let _lock = ENV_LOCK.lock().unwrap(); + let err = resolve_credential( + None, + "_JR_TEST_UNSET_MISSING", + "--email", + "Jira email", + false, + true, + None, + ) + .unwrap_err(); + let msg = err.to_string(); + assert!( + err.downcast_ref::() + .is_some_and(|e| matches!(e, JrError::UserError(_))), + "Expected JrError::UserError, got: {err}" + ); + assert!( + msg.contains("--email") && msg.contains("$_JR_TEST_UNSET_MISSING"), + "Error should cite both flag and env var: {msg}" + ); + } + + #[test] + fn resolve_credential_oauth_hint_appears_in_error() { + // Same env-read serialization as the test above. + let _lock = ENV_LOCK.lock().unwrap(); + let err = resolve_credential( + None, + "_JR_TEST_UNSET_OAUTH", + "--client-id", + "OAuth Client ID", + false, + true, + Some(OAUTH_APP_HINT), + ) + .unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("developer.atlassian.com/console/myapps"), + "OAuth error should cite dev console URL: {msg}" + ); + } } diff --git a/src/cli/init.rs b/src/cli/init.rs index 62abbae..17f2865 100644 --- a/src/cli/init.rs +++ b/src/cli/init.rs @@ -43,11 +43,14 @@ pub async fn handle() -> Result<()> { }; config.save_global()?; - // Step 3: Authenticate + // Step 3: Authenticate. `jr init` is inherently interactive (Select + // prompts above), so pass no_input=false and let dialoguer handle each + // credential prompt. Flags aren't plumbed through init — users who want + // a non-interactive setup should run `jr auth login` directly. if auth_choice == 0 { - crate::cli::auth::login_oauth().await?; + crate::cli::auth::login_oauth(None, None, false).await?; } else { - crate::cli::auth::login_token().await?; + crate::cli::auth::login_token(None, None, false).await?; let mut config = Config::load()?; config.global.instance.auth_method = Some("api_token".into()); config.save_global()?; diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 20aef62..4636e91 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -188,6 +188,20 @@ pub enum AuthCommand { /// Use OAuth 2.0 instead of API token (requires your own OAuth app) #[arg(long)] oauth: bool, + /// Jira email (API token flow). Prefer $JR_EMAIL over this flag. + #[arg(long)] + email: Option, + /// API token (API token flow). Prefer $JR_API_TOKEN over this flag — bare + /// CLI args can leak via process lists (`ps`, audit logs). + #[arg(long)] + token: Option, + /// OAuth Client ID (OAuth flow). Prefer $JR_OAUTH_CLIENT_ID over this flag. + #[arg(long)] + client_id: Option, + /// OAuth Client Secret (OAuth flow). Prefer $JR_OAUTH_CLIENT_SECRET over + /// this flag — bare CLI args can leak via process lists. + #[arg(long)] + client_secret: Option, }, /// Show authentication status Status, @@ -202,6 +216,20 @@ pub enum AuthCommand { /// Use OAuth 2.0 instead of API token (matches `jr auth login --oauth`) #[arg(long)] oauth: bool, + /// Jira email (API token flow). Prefer $JR_EMAIL over this flag. + #[arg(long)] + email: Option, + /// API token (API token flow). Prefer $JR_API_TOKEN over this flag — + /// bare CLI args can leak via process lists. + #[arg(long)] + token: Option, + /// OAuth Client ID (OAuth flow). Prefer $JR_OAUTH_CLIENT_ID over this flag. + #[arg(long)] + client_id: Option, + /// OAuth Client Secret (OAuth flow). Prefer $JR_OAUTH_CLIENT_SECRET over + /// this flag — bare CLI args can leak via process lists. + #[arg(long)] + client_secret: Option, }, } diff --git a/src/main.rs b/src/main.rs index b74689f..e27092b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -70,16 +70,37 @@ async fn run(cli: Cli) -> anyhow::Result<()> { cli::assets::handle(command, &cli.output, &client).await } cli::Command::Auth { command } => match command { - cli::AuthCommand::Login { oauth } => { + cli::AuthCommand::Login { + oauth, + email, + token, + client_id, + client_secret, + } => { if oauth { - cli::auth::login_oauth().await + cli::auth::login_oauth(client_id, client_secret, cli.no_input).await } else { - cli::auth::login_token().await + cli::auth::login_token(email, token, cli.no_input).await } } cli::AuthCommand::Status => cli::auth::status().await, - cli::AuthCommand::Refresh { oauth } => { - cli::auth::refresh_credentials(oauth, &cli.output).await + cli::AuthCommand::Refresh { + oauth, + email, + token, + client_id, + client_secret, + } => { + cli::auth::refresh_credentials( + oauth, + email, + token, + client_id, + client_secret, + cli.no_input, + &cli.output, + ) + .await } }, cli::Command::Me => { diff --git a/tests/auth_refresh.rs b/tests/auth_refresh.rs index e0bc533..49c72d5 100644 --- a/tests/auth_refresh.rs +++ b/tests/auth_refresh.rs @@ -40,13 +40,15 @@ fn auth_refresh_oauth_help_is_accepted() { } #[test] -fn auth_refresh_non_interactive_fails_without_panic() { - // Pin: with stdin closed, `jr auth refresh` must exit non-zero without - // panicking. The dialoguer prompts inside login_token hit EOF and return - // io::UnexpectedEof, which should propagate as a normal error rather - // than a panic or abort. If login gains non-interactive flag - // equivalents later, tighten this to assert the specific exit code and - // the "Credentials were cleared" recovery message. +fn auth_refresh_no_input_fails_with_clear_message() { + // Pin: `jr auth refresh --no-input` without any credential flags must + // fail with a UserError (exit 64) that names the missing flag and env + // var. Enabled by #211 — login flows now resolve credentials via + // flag → env → prompt and error explicitly under --no-input. + // + // Replaces the pre-#211 test that asserted "fails without panic" when + // stdin was closed; the new contract is stronger (specific exit code + + // actionable message) so scripts/agents can recover. // // `JR_SERVICE_NAME` scopes the keychain service so `auth::clear_credentials()` // inside the subprocess never touches the developer's real `jr-jira-cli` @@ -59,8 +61,15 @@ fn auth_refresh_non_interactive_fails_without_panic() { .env("XDG_CACHE_HOME", cache_dir.path()) .env("XDG_CONFIG_HOME", config_dir.path()) .env("JR_SERVICE_NAME", "jr-jira-cli-test") - .args(["auth", "refresh"]) - .write_stdin("") + .env_remove("JR_EMAIL") + .env_remove("JR_API_TOKEN") + // Config::load() merges JR_* via figment's Env::prefixed at + // src/config.rs:65 — JR_INSTANCE_AUTH_METHOD=oauth in the parent + // shell would flip refresh to the OAuth path and our email/JR_EMAIL + // stderr assertions would fail. Explicitly clear it to pin the + // api_token flow for this test. + .env_remove("JR_INSTANCE_AUTH_METHOD") + .args(["--no-input", "auth", "refresh"]) .output() .unwrap(); @@ -68,8 +77,26 @@ fn auth_refresh_non_interactive_fails_without_panic() { assert!( !output.status.success(), - "auth refresh with closed stdin should fail, got stdout: {}", + "auth refresh --no-input without flags should fail, got stdout: {}", String::from_utf8_lossy(&output.stdout) ); + assert_eq!( + output.status.code(), + Some(64), + "Missing credentials under --no-input should exit 64 (UserError), got: {:?}", + output.status.code() + ); + assert!( + stderr.contains("--email") && stderr.contains("$JR_EMAIL"), + "Error should cite --email flag and $JR_EMAIL env var: {stderr}" + ); + // The clear-then-login ordering means credentials *are* cleared before + // the login failure bubbles up. The recovery hint tells users exactly + // how to get back to a working state — pinning it here so a future + // refactor can't silently drop the guidance. + assert!( + stderr.contains("Credentials were cleared"), + "Error should include recovery hint after cleared credentials: {stderr}" + ); assert!(!stderr.contains("panic"), "stderr leaked a panic: {stderr}"); }