From 2772568d26617a77aae846c427d4797997482c50 Mon Sep 17 00:00:00 2001 From: euxaristia Date: Thu, 18 Jun 2026 00:14:18 -0400 Subject: [PATCH] cli: raise error when auth login remote URL is missing Avoid a silent/hanging failure when running 'lore auth login' without a remote URL argument or configured repository remote. Add CLI-side validation and comprehensive unit tests to ensure a clear error is printed to the user. Signed-off-by: euxaristia --- lore-client/src/cli/commands/auth.rs | 128 ++++++++++++++++++++++++++- lore/src/auth.rs | 51 ++++++----- 2 files changed, 152 insertions(+), 27 deletions(-) diff --git a/lore-client/src/cli/commands/auth.rs b/lore-client/src/cli/commands/auth.rs index 063b5459..573020ad 100644 --- a/lore-client/src/cli/commands/auth.rs +++ b/lore-client/src/cli/commands/auth.rs @@ -38,6 +38,8 @@ use crate::println; use crate::styling::CommonStyles; use crate::util; +const LOGIN_REMOTE_REQUIRED: &str = "No remote URL supplied. Run `lore auth login ` or run from a Lore repository with a configured remote."; + #[derive(Args)] pub struct AuthArgs { #[command(subcommand)] @@ -112,8 +114,58 @@ pub enum AuthCommands { Clear, } +fn configured_remote_url(repository_path: &str) -> Option { + lore_revision::repository::load_repository_config(repository_path) + .ok() + .and_then(|config| config.remote_url) + .filter(|remote_url| !remote_url.is_empty()) +} + +fn login_with_token_can_use_explicit_auth_url(args: &AuthLoginArgs) -> bool { + args.token_type.is_some() + && args.token.is_some() + && args + .auth_url + .as_deref() + .is_some_and(|auth_url| !auth_url.is_empty()) +} + +fn resolve_login_remote_url( + globals: &LoreGlobalArgs, + args: &AuthLoginArgs, +) -> Result { + if let Some(remote_url) = args + .remote_url + .as_deref() + .filter(|remote_url| !remote_url.is_empty()) + { + return Ok(LoreString::from(remote_url)); + } + + if let Some(remote_url) = configured_remote_url(globals.repository_path.as_str()) { + return Ok(LoreString::from(remote_url.as_str())); + } + + if login_with_token_can_use_explicit_auth_url(args) { + return Ok(LoreString::default()); + } + + Err(LOGIN_REMOTE_REQUIRED) +} + pub fn handle_login_command(globals: LoreGlobalArgs, args: &AuthLoginArgs) -> u8 { - let remote_url = LoreString::from(&args.remote_url); + if args.token_type.is_some() ^ args.token.is_some() { + crate::eprintln!("Both --token-type and --token are required for non-interactive login"); + return 1; + } + + let remote_url = match resolve_login_remote_url(&globals, args) { + Ok(remote_url) => remote_url, + Err(message) => { + crate::eprintln!("{message}"); + return 1; + } + }; let callback = output_formatter().unwrap_or(Some( (Box::new(move |event: &LoreEvent| match event { @@ -148,9 +200,6 @@ pub fn handle_login_command(globals: LoreGlobalArgs, args: &AuthLoginArgs) -> u8 auth_url: args.auth_url.as_deref().into(), }; runtime().block_on(auth::login_with_token(globals, args, callback)) as u8 - } else if args.token_type.is_some() || args.token.is_some() { - crate::eprintln!("Both --token-type and --token are required for non-interactive login"); - 1 } else { let args = LoreAuthLoginInteractiveArgs { remote_url, @@ -501,3 +550,74 @@ pub fn resolve_user_ids( Err(_) => HashMap::default(), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn globals_without_repo() -> LoreGlobalArgs { + let repository_path = std::env::temp_dir().join(format!( + "lore-auth-login-missing-repo-{}", + std::process::id() + )); + + LoreGlobalArgs { + repository_path: repository_path.display().to_string().into(), + ..Default::default() + } + } + + fn login_args( + remote_url: Option<&str>, + token_type: Option<&str>, + token: Option<&str>, + auth_url: Option<&str>, + ) -> AuthLoginArgs { + AuthLoginArgs { + token_type: token_type.map(str::to_owned), + token: token.map(str::to_owned), + auth_url: auth_url.map(str::to_owned), + remote_url: remote_url.map(str::to_owned), + no_browser: false, + } + } + + #[test] + fn interactive_login_without_remote_errors() { + let globals = globals_without_repo(); + let args = login_args(None, None, None, None); + + assert_eq!( + resolve_login_remote_url(&globals, &args).unwrap_err(), + LOGIN_REMOTE_REQUIRED + ); + } + + #[test] + fn token_login_with_auth_url_allows_missing_remote() { + let globals = globals_without_repo(); + let args = login_args( + None, + Some("api-key"), + Some("token-value"), + Some("ucs-auth://auth.example.com"), + ); + + assert!( + resolve_login_remote_url(&globals, &args) + .unwrap() + .is_empty() + ); + } + + #[test] + fn login_uses_remote_argument_when_present() { + let globals = globals_without_repo(); + let args = login_args(Some("lore.example.com"), None, None, None); + + assert_eq!( + resolve_login_remote_url(&globals, &args).unwrap().as_str(), + "lore.example.com" + ); + } +} diff --git a/lore/src/auth.rs b/lore/src/auth.rs index 6698f115..5622fca9 100644 --- a/lore/src/auth.rs +++ b/lore/src/auth.rs @@ -8,6 +8,7 @@ use lore_credential::UserInfo; use lore_error_set::prelude::*; use lore_macro::LoreArgs; use lore_revision::auth; +use lore_revision::auth::login::InteractiveLoginError; use lore_revision::auth::login::LoginError; use lore_revision::auth::userinfo::LoreAuthIdentityEventData; use lore_revision::auth::userinfo::LoreAuthUserInfoEventData; @@ -29,6 +30,8 @@ use crate::call::setup_execution; use crate::call_delegation::dispatch_call; use crate::interface::LoreString; +const LOGIN_REMOTE_REQUIRED: &str = "No remote URL supplied. Run `lore auth login ` or run from a Lore repository with a configured remote."; + #[error_set] pub enum AuthStoreError { TokenNotFound, @@ -206,21 +209,22 @@ async fn login_with_token_local( let auth_url: Option = args.auth_url.into(); - LORE_CONTEXT + let result = LORE_CONTEXT .scope(execution, async move { - let result = async move { - login_with_token_impl( - remote_url.as_str(), - args.token.as_str(), - args.token_type.as_str(), - auth_url.as_deref(), - ) - .await + if remote_url.is_empty() && auth_url.as_deref().unwrap_or_default().is_empty() { + return Err(LoginError::internal(LOGIN_REMOTE_REQUIRED)); } - .await; - execution_context().dispatcher.complete_result(result).await + + login_with_token_impl( + remote_url.as_str(), + args.token.as_str(), + args.token_type.as_str(), + auth_url.as_deref(), + ) + .await }) - .await + .await; + execution_context().dispatcher.complete_result(result).await } async fn login_with_token_impl( @@ -291,21 +295,22 @@ async fn login_interactive_local( let execution = setup_execution(globals, callback); - LORE_CONTEXT + let result = LORE_CONTEXT .scope(execution, async move { - let result = async move { - match auth::login::interactive(remote_url.as_str(), args.no_browser != 0).await { - Ok(user_info) => { - send_user_info(user_info); - Ok(()) - } - Err(err) => Err(err), + if remote_url.is_empty() { + return Err(InteractiveLoginError::internal(LOGIN_REMOTE_REQUIRED)); + } + + match auth::login::interactive(remote_url.as_str(), args.no_browser != 0).await { + Ok(user_info) => { + send_user_info(user_info); + Ok(()) } + Err(err) => Err(err), } - .await; - execution_context().dispatcher.complete_result(result).await }) - .await + .await; + execution_context().dispatcher.complete_result(result).await } /// Arguments for listing all stored authentication identities across endpoints.