fix(auth): all-provider auth status and scoped logout
- auth status shows every known provider with config/keyring/env status - auth status --provider <id> shows detailed single-provider info - auth list now probes keyring for all providers (was only active) - /logout clears only the active provider's key (was clearing all) - add clear_active_provider_api_key for scoped TOML key removal - add Huggingface to ProviderArg enum - add auth status tests for all-provider and scoped views Fixes #2716
This commit is contained in:
+177
-17
@@ -39,6 +39,7 @@ enum ProviderArg {
|
||||
Sglang,
|
||||
Vllm,
|
||||
Ollama,
|
||||
Huggingface,
|
||||
}
|
||||
|
||||
impl From<ProviderArg> for ProviderKind {
|
||||
@@ -60,6 +61,7 @@ impl From<ProviderArg> for ProviderKind {
|
||||
ProviderArg::Sglang => ProviderKind::Sglang,
|
||||
ProviderArg::Vllm => ProviderKind::Vllm,
|
||||
ProviderArg::Ollama => ProviderKind::Ollama,
|
||||
ProviderArg::Huggingface => ProviderKind::Huggingface,
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -298,7 +300,13 @@ struct AuthArgs {
|
||||
#[derive(Debug, Subcommand)]
|
||||
enum AuthCommand {
|
||||
/// Show current provider and credential source state.
|
||||
Status,
|
||||
/// Without `--provider`, shows all known providers.
|
||||
/// With `--provider`, shows detailed status for that provider.
|
||||
Status {
|
||||
/// Show status for a specific provider only.
|
||||
#[arg(long, value_enum)]
|
||||
provider: Option<ProviderArg>,
|
||||
},
|
||||
/// Save an API key to the shared user config file. Reads from
|
||||
/// `--api-key`, `--api-key-stdin`, or prompts on stdin when
|
||||
/// neither is given. Does not echo the key.
|
||||
@@ -898,8 +906,67 @@ fn clear_provider_api_key_from_keyring(secrets: &Secrets, provider: ProviderKind
|
||||
let _ = secrets.delete(provider_slot(provider));
|
||||
}
|
||||
|
||||
fn auth_status_lines(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
|
||||
let provider = store.config.provider;
|
||||
fn auth_status_all_providers(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
|
||||
let active_provider = store.config.provider;
|
||||
let mut lines = Vec::new();
|
||||
lines.push(format!(
|
||||
"active provider: {} (set via config or CODEWHALE_PROVIDER)",
|
||||
active_provider.as_str()
|
||||
));
|
||||
lines.push(String::new());
|
||||
lines.push(format!(
|
||||
"{:<14} {:<8} {:<10} {:<8} {}",
|
||||
"provider", "config", "keyring", "env", "status"
|
||||
));
|
||||
lines.push(format!("{}", "-".repeat(70)));
|
||||
|
||||
for provider in PROVIDER_LIST {
|
||||
let config_key = provider_config_api_key(store, provider);
|
||||
let keyring_key = provider_keyring_api_key(secrets, provider);
|
||||
let env_key = provider_env_value(provider);
|
||||
|
||||
let config_status = config_key.map(|_| "set").unwrap_or("-");
|
||||
let keyring_status = keyring_key.as_ref().map(|_| "set").unwrap_or("-");
|
||||
let env_status = env_key.as_ref().map(|_| "set").unwrap_or("-");
|
||||
|
||||
let source = if config_key.is_some() {
|
||||
"config"
|
||||
} else if keyring_key.is_some() {
|
||||
"keyring"
|
||||
} else if env_key.is_some() {
|
||||
"env"
|
||||
} else {
|
||||
"unset"
|
||||
};
|
||||
|
||||
let active_marker = if provider == active_provider {
|
||||
" *"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
|
||||
lines.push(format!(
|
||||
"{:<14} {:<8} {:<10} {:<8} {}{}",
|
||||
provider.as_str(),
|
||||
config_status,
|
||||
keyring_status,
|
||||
env_status,
|
||||
source,
|
||||
active_marker
|
||||
));
|
||||
}
|
||||
|
||||
lines.push(String::new());
|
||||
lines.push("* = active provider (from config or CODEWHALE_PROVIDER)".to_string());
|
||||
lines.push("Run `codewhale auth status --provider <id>` for detailed info.".to_string());
|
||||
lines
|
||||
}
|
||||
|
||||
fn auth_status_lines_for_provider(
|
||||
store: &ConfigStore,
|
||||
secrets: &Secrets,
|
||||
provider: ProviderKind,
|
||||
) -> Vec<String> {
|
||||
let config_key = provider_config_api_key(store, provider);
|
||||
let keyring_key = provider_keyring_api_key(secrets, provider);
|
||||
let env_key = provider_env_value(provider);
|
||||
@@ -930,8 +997,11 @@ fn auth_status_lines(store: &ConfigStore, secrets: &Secrets) -> Vec<String> {
|
||||
.map(|(_, value)| format!("set, last4: {}", last4_label(value)))
|
||||
.unwrap_or_else(|| "unset".to_string());
|
||||
|
||||
let is_active = provider == store.config.provider;
|
||||
let active_marker = if is_active { " (active provider)" } else { "" };
|
||||
|
||||
vec![
|
||||
format!("provider: {}", provider.as_str()),
|
||||
format!("provider: {}{}", provider.as_str(), active_marker),
|
||||
format!(
|
||||
"auth mode: {}",
|
||||
store.config.auth_mode.as_deref().unwrap_or("api_key")
|
||||
@@ -978,9 +1048,19 @@ fn run_auth_command_with_secrets(
|
||||
secrets: &Secrets,
|
||||
) -> Result<()> {
|
||||
match command {
|
||||
AuthCommand::Status => {
|
||||
for line in auth_status_lines(store, secrets) {
|
||||
println!("{line}");
|
||||
AuthCommand::Status { provider } => {
|
||||
match provider {
|
||||
Some(p) => {
|
||||
let provider: ProviderKind = p.into();
|
||||
for line in auth_status_lines_for_provider(store, secrets, provider) {
|
||||
println!("{line}");
|
||||
}
|
||||
}
|
||||
None => {
|
||||
for line in auth_status_all_providers(store, secrets) {
|
||||
println!("{line}");
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -1056,12 +1136,10 @@ fn run_auth_command_with_secrets(
|
||||
}
|
||||
AuthCommand::List => {
|
||||
println!("provider config store env active");
|
||||
let active_provider = store.config.provider;
|
||||
for provider in PROVIDER_LIST {
|
||||
let slot = provider_slot(provider);
|
||||
let file = provider_config_set(store, provider);
|
||||
let keyring = (provider == active_provider && !file)
|
||||
.then(|| provider_keyring_set(secrets, provider));
|
||||
let keyring = (!file).then(|| provider_keyring_set(secrets, provider));
|
||||
let env = provider_env_set(provider);
|
||||
let active = if file {
|
||||
"config"
|
||||
@@ -2479,7 +2557,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auth_status_and_list_only_probe_active_provider_keyring() {
|
||||
fn auth_status_scoped_probe_and_list_all_provider_keyrings() {
|
||||
use codewhale_secrets::{KeyringStore, SecretsError};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
@@ -2517,14 +2595,29 @@ mod tests {
|
||||
let inner = Arc::new(RecordingStore::default());
|
||||
let secrets = Secrets::new(inner.clone());
|
||||
|
||||
run_auth_command_with_secrets(&mut store, AuthCommand::Status, &secrets)
|
||||
.expect("status should succeed");
|
||||
run_auth_command_with_secrets(
|
||||
&mut store,
|
||||
AuthCommand::Status {
|
||||
provider: Some(ProviderArg::Deepseek),
|
||||
},
|
||||
&secrets,
|
||||
)
|
||||
.expect("status should succeed");
|
||||
run_auth_command_with_secrets(&mut store, AuthCommand::List, &secrets)
|
||||
.expect("list should succeed");
|
||||
|
||||
assert_eq!(
|
||||
inner.gets.lock().unwrap().as_slice(),
|
||||
["deepseek", "deepseek"]
|
||||
let probed = inner.gets.lock().unwrap();
|
||||
// Scoped status probes only the requested provider.
|
||||
assert_eq!(probed[0], "deepseek");
|
||||
// List now probes all providers (not just active) to fix the
|
||||
// stale keyring-only-for-active-provider bug.
|
||||
assert!(probed.len() > 1, "list should probe all providers");
|
||||
assert!(
|
||||
PROVIDER_LIST
|
||||
.iter()
|
||||
.all(|p| probed.contains(&provider_slot(*p).to_string())),
|
||||
"every known provider should be probed by auth list: {:?}",
|
||||
*probed
|
||||
);
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
@@ -2552,7 +2645,8 @@ mod tests {
|
||||
inner.set("deepseek", "sk-keyring-2222").unwrap();
|
||||
let secrets = Secrets::new(inner);
|
||||
|
||||
let output = auth_status_lines(&store, &secrets).join("\n");
|
||||
let output =
|
||||
auth_status_lines_for_provider(&store, &secrets, ProviderKind::Deepseek).join("\n");
|
||||
|
||||
assert!(output.contains("provider: deepseek"));
|
||||
assert!(output.contains("active source: config (last4: ...3333)"));
|
||||
@@ -2568,6 +2662,72 @@ mod tests {
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auth_status_all_providers_lists_every_known_provider() {
|
||||
use codewhale_secrets::{InMemoryKeyringStore, KeyringStore};
|
||||
use std::sync::Arc;
|
||||
|
||||
let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default();
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"deepseek-cli-auth-all-status-test-{}-{nanos}.toml",
|
||||
std::process::id()
|
||||
));
|
||||
let mut store = ConfigStore::load(Some(path.clone())).expect("store should load");
|
||||
store.config.provider = ProviderKind::Deepseek;
|
||||
store.config.providers.arcee.api_key = Some("sk-arcee-test1234".to_string());
|
||||
|
||||
let inner = Arc::new(InMemoryKeyringStore::new());
|
||||
inner.set("openrouter", "sk-or-test5678").unwrap();
|
||||
let secrets = Secrets::new(inner);
|
||||
|
||||
let output = auth_status_all_providers(&store, &secrets).join("\n");
|
||||
|
||||
// Should list all known providers
|
||||
assert!(output.contains("deepseek"));
|
||||
assert!(output.contains("arcee"));
|
||||
assert!(output.contains("openrouter"));
|
||||
assert!(output.contains("huggingface"));
|
||||
assert!(output.contains("ollama"));
|
||||
|
||||
// Active provider should be marked
|
||||
assert!(output.contains("deepseek") && output.contains("*"));
|
||||
|
||||
// Arcee should show config source
|
||||
assert!(output.contains("config"));
|
||||
|
||||
// Should NOT leak raw keys
|
||||
assert!(!output.contains("sk-arcee-test1234"));
|
||||
assert!(!output.contains("sk-or-test5678"));
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auth_status_scoped_provider_shows_detailed_info() {
|
||||
use codewhale_secrets::InMemoryKeyringStore;
|
||||
use std::sync::Arc;
|
||||
|
||||
let nanos = chrono::Utc::now().timestamp_nanos_opt().unwrap_or_default();
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"deepseek-cli-auth-scoped-test-{}-{nanos}.toml",
|
||||
std::process::id()
|
||||
));
|
||||
let mut store = ConfigStore::load(Some(path.clone())).expect("store should load");
|
||||
store.config.provider = ProviderKind::Deepseek;
|
||||
store.config.providers.arcee.api_key = Some("sk-arcee-9999".to_string());
|
||||
|
||||
let secrets = Secrets::new(Arc::new(InMemoryKeyringStore::new()));
|
||||
|
||||
let output =
|
||||
auth_status_lines_for_provider(&store, &secrets, ProviderKind::Arcee).join("\n");
|
||||
|
||||
assert!(output.contains("provider: arcee"));
|
||||
assert!(output.contains("active source: config (last4: ...9999)"));
|
||||
assert!(!output.contains("sk-arcee-9999"));
|
||||
|
||||
let _ = std::fs::remove_file(path);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dispatch_keyring_recovery_self_heals_into_config_file() {
|
||||
use codewhale_secrets::{InMemoryKeyringStore, KeyringStore};
|
||||
|
||||
@@ -7,8 +7,8 @@ use super::CommandResult;
|
||||
use crate::client::DeepSeekClient;
|
||||
use crate::config::{
|
||||
ApiProvider, COMMON_DEEPSEEK_MODELS, Config, DEFAULT_XIAOMI_MIMO_BASE_URL,
|
||||
XIAOMI_MIMO_PAY_AS_YOU_GO_BASE_URL, clear_api_key, effective_home_dir, expand_path,
|
||||
normalize_model_name_for_provider,
|
||||
XIAOMI_MIMO_PAY_AS_YOU_GO_BASE_URL, clear_active_provider_api_key, effective_home_dir,
|
||||
expand_path, normalize_model_name_for_provider,
|
||||
};
|
||||
use crate::config_ui::{ConfigUiMode, parse_mode};
|
||||
use crate::llm_client::LlmClient;
|
||||
@@ -1600,17 +1600,25 @@ pub fn lsp_command(app: &mut App, arg: Option<&str>) -> CommandResult {
|
||||
}
|
||||
}
|
||||
|
||||
/// Logout - clear API key and return to onboarding
|
||||
/// Logout - clear all saved API keys and return to onboarding.
|
||||
/// This is NOT provider-scoped — it clears keys for every saved provider.
|
||||
/// For single-provider key replacement, use
|
||||
/// `codewhale auth clear --provider <id>` and
|
||||
/// `codewhale auth set --provider <id>`.
|
||||
pub fn logout(app: &mut App) -> CommandResult {
|
||||
match clear_api_key() {
|
||||
let provider_name = app.api_provider.as_str();
|
||||
match clear_active_provider_api_key(provider_name) {
|
||||
Ok(()) => {
|
||||
app.onboarding = OnboardingState::ApiKey;
|
||||
app.onboarding_needs_api_key = true;
|
||||
app.api_key_input.clear();
|
||||
app.api_key_cursor = 0;
|
||||
CommandResult::message("Logged out. Enter a new API key to continue.")
|
||||
CommandResult::message(format!(
|
||||
"Cleared API key for {provider_name}. \
|
||||
Use `codewhale auth clear --provider <id>` to clear a different provider."
|
||||
))
|
||||
}
|
||||
Err(e) => CommandResult::error(format!("Failed to clear API key: {e}")),
|
||||
Err(e) => CommandResult::error(format!("Failed to clear API key for {provider_name}: {e}")),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5319,6 +5319,69 @@ pub fn clear_api_key() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Clear only the active provider's API key from the config file.
|
||||
/// Unlike `clear_api_key()` which strips ALL api_key lines, this
|
||||
/// removes only the key for the specified provider section.
|
||||
pub fn clear_active_provider_api_key(provider: &str) -> Result<()> {
|
||||
let config_path = default_config_path()
|
||||
.context("Failed to resolve config path: home directory not found.")?;
|
||||
|
||||
if !config_path.exists() {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
let existing = fs::read_to_string(&config_path)?;
|
||||
let mut result = String::new();
|
||||
let target_section = format!("[providers.{}]", provider);
|
||||
let mut in_target_section = false;
|
||||
|
||||
for line in existing.lines() {
|
||||
let trimmed = line.trim();
|
||||
|
||||
// Track which [providers.X] section we're in.
|
||||
if trimmed.starts_with("[providers.") {
|
||||
in_target_section = trimmed == target_section;
|
||||
} else if trimmed.starts_with('[') {
|
||||
in_target_section = false;
|
||||
}
|
||||
|
||||
// For the root section (before any [headers]), clear api_key
|
||||
// only if the provider is "deepseek" (root-level key).
|
||||
let is_root_key = !in_target_section
|
||||
&& provider == "deepseek"
|
||||
&& trimmed.strip_prefix("api_key").is_some_and(|rest| {
|
||||
let rest = rest.trim_start();
|
||||
rest.is_empty() || rest.starts_with('=')
|
||||
});
|
||||
|
||||
// For a provider section, clear api_key if we're in the target section.
|
||||
let is_provider_key = in_target_section
|
||||
&& trimmed.strip_prefix("api_key").is_some_and(|rest| {
|
||||
let rest = rest.trim_start();
|
||||
rest.is_empty() || rest.starts_with('=')
|
||||
});
|
||||
|
||||
if is_root_key || is_provider_key {
|
||||
continue;
|
||||
}
|
||||
result.push_str(line);
|
||||
result.push('\n');
|
||||
}
|
||||
|
||||
write_config_file_secure(&config_path, &result)
|
||||
.with_context(|| format!("Failed to write config to {}", config_path.display()))?;
|
||||
log_sensitive_event(
|
||||
"credential.clear",
|
||||
json!({
|
||||
"backend": "config_file",
|
||||
"config_path": config_path.display().to_string(),
|
||||
"scope": provider,
|
||||
}),
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -6199,20 +6199,13 @@ async fn execute_command_input(
|
||||
// (#343). The on-disk side is handled by clear_api_key() inside
|
||||
// commands::config::logout.
|
||||
if input.trim().eq_ignore_ascii_case("/logout") {
|
||||
// Only clear the active provider's in-memory API key, not every
|
||||
// provider. The on-disk clear_api_key() inside commands::config::logout
|
||||
// already removes all saved keys; clearing only the active slot here
|
||||
// prevents surprising side-effects when the user has multiple providers
|
||||
// configured.
|
||||
config.api_key = None;
|
||||
if let Some(providers) = config.providers.as_mut() {
|
||||
providers.deepseek.api_key = None;
|
||||
providers.deepseek_cn.api_key = None;
|
||||
providers.nvidia_nim.api_key = None;
|
||||
providers.openai.api_key = None;
|
||||
providers.atlascloud.api_key = None;
|
||||
providers.openrouter.api_key = None;
|
||||
providers.novita.api_key = None;
|
||||
providers.fireworks.api_key = None;
|
||||
providers.sglang.api_key = None;
|
||||
providers.vllm.api_key = None;
|
||||
providers.ollama.api_key = None;
|
||||
}
|
||||
config.provider_config_for_mut(app.api_provider).api_key = None;
|
||||
app.api_key_env_only = crate::config::active_provider_uses_env_only_api_key(config);
|
||||
}
|
||||
apply_command_result(
|
||||
|
||||
Reference in New Issue
Block a user