diff --git a/crates/cli/src/commands/alias.rs b/crates/cli/src/commands/alias.rs index 6f8c096..04e1793 100644 --- a/crates/cli/src/commands/alias.rs +++ b/crates/cli/src/commands/alias.rs @@ -8,7 +8,8 @@ use serde::Serialize; use crate::exit_code::ExitCode; use crate::output::{Formatter, OutputConfig}; -use rc_core::{Alias, AliasManager, validate_alias_endpoint}; +use rc_core::{Alias, AliasManager, Error, ObjectStore as _, validate_alias_endpoint}; +use rc_s3::S3Client; /// Alias subcommands for managing storage service connections #[derive(Subcommand, Debug)] @@ -215,6 +216,12 @@ async fn execute_set(args: SetArgs, manager: &AliasManager, formatter: &Formatte alias.bucket_lookup = args.bucket_lookup; alias.insecure = args.insecure; + if let Err(e) = validate_alias_credentials(&alias).await { + let code = exit_code_from_error(&e); + formatter.error_with_code(code, &e.to_string()); + return code; + } + // Save alias match manager.set(alias) { Ok(()) => { @@ -239,6 +246,44 @@ async fn execute_set(args: SetArgs, manager: &AliasManager, formatter: &Formatte } } +async fn validate_alias_credentials(alias: &Alias) -> rc_core::Result<()> { + if alias.anonymous { + return Ok(()); + } + + let client = S3Client::new(alias.clone()).await?; + match client.list_buckets().await { + Ok(_) => Ok(()), + Err(error) => { + let message = error.to_string(); + if is_alias_auth_validation_failure(&message) { + Err(Error::Auth("Invalid access key or secret key".to_string())) + } else if is_alias_authorization_only_failure(&message) { + Ok(()) + } else { + Err(error) + } + } + } +} + +fn is_alias_auth_validation_failure(message: &str) -> bool { + [ + "InvalidAccessKeyId", + "SignatureDoesNotMatch", + "InvalidToken", + "ExpiredToken", + ] + .iter() + .any(|code| message.contains(code)) +} + +fn is_alias_authorization_only_failure(message: &str) -> bool { + ["AccessDenied", "AllAccessDisabled"] + .iter() + .any(|code| message.contains(code)) +} + async fn execute_list(args: ListArgs, manager: &AliasManager, formatter: &Formatter) -> ExitCode { match manager.list() { Ok(aliases) => { @@ -336,6 +381,8 @@ fn exit_code_from_error(error: &rc_core::Error) -> ExitCode { #[cfg(test)] mod tests { use super::*; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; #[test] fn test_set_args_defaults() { @@ -377,19 +424,7 @@ mod tests { temp_dir.path().join("config.toml"), )); let formatter = Formatter::new(OutputConfig::default()); - let args = SetArgs { - name: "rustfs".to_string(), - endpoint: "http://rustfs-node{1...32}:9000".to_string(), - access_key: Some("accesskey".to_string()), - secret_key: Some("secretkey".to_string()), - anonymous: false, - client_cert: None, - client_key: None, - region: "us-east-1".to_string(), - signature: "v4".to_string(), - bucket_lookup: "auto".to_string(), - insecure: false, - }; + let args = set_args("http://rustfs-node{1...32}:9000"); let exit_code = execute_set(args, &manager, &formatter).await; @@ -397,6 +432,46 @@ mod tests { assert!(manager.get("rustfs").is_err()); } + #[tokio::test] + async fn test_execute_set_rejects_invalid_credentials() { + let endpoint = spawn_s3_error_server("InvalidAccessKeyId").await; + let temp_dir = tempfile::TempDir::new().unwrap(); + let manager = AliasManager::with_config_manager(rc_core::ConfigManager::with_path( + temp_dir.path().join("config.toml"), + )); + let formatter = Formatter::new(OutputConfig::default()); + + let exit_code = execute_set(set_args(&endpoint), &manager, &formatter).await; + + assert_eq!(exit_code, ExitCode::AuthError); + assert!(manager.get("rustfs").is_err()); + } + + #[tokio::test] + async fn test_execute_set_allows_authenticated_access_denied() { + let endpoint = spawn_s3_error_server("AccessDenied").await; + let temp_dir = tempfile::TempDir::new().unwrap(); + let manager = AliasManager::with_config_manager(rc_core::ConfigManager::with_path( + temp_dir.path().join("config.toml"), + )); + let formatter = Formatter::new(OutputConfig::default()); + + let exit_code = execute_set(set_args(&endpoint), &manager, &formatter).await; + + assert_eq!(exit_code, ExitCode::Success); + assert!(manager.get("rustfs").is_ok()); + } + + #[test] + fn test_alias_auth_validation_preserves_signing_configuration_errors() { + assert!(is_alias_auth_validation_failure("InvalidAccessKeyId")); + assert!(is_alias_auth_validation_failure("SignatureDoesNotMatch")); + assert!(!is_alias_auth_validation_failure( + "AuthorizationHeaderMalformed" + )); + assert!(!is_alias_auth_validation_failure("RequestTimeTooSkewed")); + } + #[test] fn test_alias_endpoint_error_message_omits_config_prefix() { let message = alias_endpoint_error_message(rc_core::Error::Config( @@ -405,4 +480,42 @@ mod tests { assert_eq!(message, "Endpoint must include a host"); } + + fn set_args(endpoint: &str) -> SetArgs { + SetArgs { + name: "rustfs".to_string(), + endpoint: endpoint.to_string(), + access_key: Some("accesskey".to_string()), + secret_key: Some("secretkey".to_string()), + anonymous: false, + client_cert: None, + client_key: None, + region: "us-east-1".to_string(), + signature: "v4".to_string(), + bucket_lookup: "auto".to_string(), + insecure: false, + } + } + + async fn spawn_s3_error_server(code: &'static str) -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + tokio::spawn(async move { + for _ in 0..3 { + let Ok((mut stream, _)) = listener.accept().await else { + return; + }; + let mut request = [0_u8; 4096]; + let _ = stream.read(&mut request).await; + let body = format!("{code}test"); + let response = format!( + "HTTP/1.1 403 Forbidden\r\ncontent-type: application/xml\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + body.len(), + body + ); + let _ = stream.write_all(response.as_bytes()).await; + } + }); + format!("http://{addr}") + } } diff --git a/crates/cli/tests/error_contract.rs b/crates/cli/tests/error_contract.rs index 32bb0f2..c3b4c7b 100644 --- a/crates/cli/tests/error_contract.rs +++ b/crates/cli/tests/error_contract.rs @@ -3,8 +3,11 @@ //! These tests cover local validation paths that do not require a running S3 //! backend but still need stable JSON error metadata. +use std::io::{Read, Write}; +use std::net::TcpListener; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; +use std::thread; fn rc_binary() -> PathBuf { if let Ok(path) = std::env::var("CARGO_BIN_EXE_rc") { @@ -53,6 +56,30 @@ fn run_rc_with_config(args: &[&str], config_dir: &Path) -> Output { .expect("failed to execute rc") } +fn spawn_s3_error_server(code: &'static str) -> String { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind mock S3 server"); + let address = listener.local_addr().expect("mock S3 server address"); + + thread::spawn(move || { + for _ in 0..3 { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + let mut request = [0_u8; 4096]; + let _ = stream.read(&mut request); + let body = format!("{code}test"); + let response = format!( + "HTTP/1.1 403 Forbidden\r\ncontent-type: application/xml\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{}", + body.len(), + body + ); + let _ = stream.write_all(response.as_bytes()); + } + }); + + format!("http://{address}") +} + #[test] fn cp_local_to_local_json_error_reports_usage_metadata() { let temp_dir = tempfile::tempdir().expect("create temp dir"); @@ -203,6 +230,62 @@ fn alias_set_json_error_rejects_invalid_signature() { ); } +#[test] +#[cfg(not(windows))] +fn alias_set_json_error_rejects_invalid_credentials_without_saving_alias() { + let config_dir = tempfile::tempdir().expect("create temp config dir"); + let endpoint = spawn_s3_error_server("InvalidAccessKeyId"); + + let output = run_rc_with_config( + &[ + "alias", + "set", + "bad", + &endpoint, + "accesskey", + "secretkey", + "--json", + ], + config_dir.path(), + ); + + assert_eq!( + output.status.code(), + Some(4), + "stdout: {}\nstderr: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + output.stdout.is_empty(), + "auth JSON errors should be emitted on stderr" + ); + + let stderr = String::from_utf8(output.stderr).expect("stderr should be UTF-8"); + let json: serde_json::Value = serde_json::from_str(&stderr).expect("stderr is valid JSON"); + assert_eq!( + json["error"], + "Authentication failed: Invalid access key or secret key" + ); + assert_eq!(json["code"], 4); + assert_eq!(json["details"]["type"], "auth_error"); + assert_eq!(json["details"]["retryable"], false); + + let list_output = run_rc_with_config(&["alias", "list", "--json"], config_dir.path()); + assert!( + list_output.status.success(), + "stderr: {}", + String::from_utf8_lossy(&list_output.stderr) + ); + + let stdout = String::from_utf8(list_output.stdout).expect("stdout should be UTF-8"); + let payload: serde_json::Value = serde_json::from_str(&stdout).expect("stdout is valid JSON"); + assert_eq!( + payload["aliases"].as_array().expect("aliases array").len(), + 0 + ); +} + #[test] fn alias_set_anonymous_accepts_missing_credentials_and_lists_auth_mode() { let config_dir = tempfile::tempdir().expect("create temp config dir"); diff --git a/crates/cli/tests/golden.rs b/crates/cli/tests/golden.rs index 33ad996..bea5ac7 100644 --- a/crates/cli/tests/golden.rs +++ b/crates/cli/tests/golden.rs @@ -70,8 +70,7 @@ mod alias_tests { "set", "test-alias", "http://localhost:9000", - "accesskey", - "secretkey", + "--anonymous", "--json", ]) .env("RC_CONFIG_DIR", config_dir) @@ -100,8 +99,7 @@ mod alias_tests { "set", "local", "http://localhost:9000", - "accesskey", - "secretkey", + "--anonymous", "--json", ]) .env("RC_CONFIG_DIR", config_dir) @@ -114,8 +112,7 @@ mod alias_tests { "set", "s3", "https://s3.amazonaws.com", - "awskey", - "awssecret", + "--anonymous", "--region", "us-west-2", "--json", @@ -156,8 +153,7 @@ mod alias_tests { "set", "to-remove", "http://localhost:9000", - "accesskey", - "secretkey", + "--anonymous", "--json", ]) .env("RC_CONFIG_DIR", config_dir) diff --git a/crates/cli/tests/ls_versions_error.rs b/crates/cli/tests/ls_versions_error.rs index e5cbf09..d3228f3 100644 --- a/crates/cli/tests/ls_versions_error.rs +++ b/crates/cli/tests/ls_versions_error.rs @@ -2,7 +2,7 @@ use std::net::TcpListener; use std::path::PathBuf; -use std::process::{Command, Output}; +use std::process::Command; fn rc_binary() -> PathBuf { if let Ok(path) = std::env::var("CARGO_BIN_EXE_rc") { @@ -24,14 +24,6 @@ fn rc_binary() -> PathBuf { workspace_root.join("target/release/rc") } -fn run_rc(args: &[&str], config_dir: &std::path::Path) -> Output { - Command::new(rc_binary()) - .args(args) - .env("RC_CONFIG_DIR", config_dir) - .output() - .expect("run rc command") -} - fn unused_local_endpoint() -> String { let listener = TcpListener::bind("127.0.0.1:0").expect("bind local endpoint"); let address = listener.local_addr().expect("local endpoint address"); @@ -44,29 +36,18 @@ fn ls_versions_json_network_error_reports_exit_code() { let config_dir = tempfile::tempdir().expect("create config dir"); let endpoint = unused_local_endpoint(); - let alias_output = run_rc( - &[ - "alias", - "set", - "test", - &endpoint, - "accesskey", - "secretkey", - "--bucket-lookup", - "path", - ], - config_dir.path(), - ); - assert!( - alias_output.status.success(), - "alias setup failed: {}", - String::from_utf8_lossy(&alias_output.stderr) - ); - - let output = run_rc( - &["--json", "ls", "test/bucket", "--versions"], - config_dir.path(), - ); + let output = Command::new(rc_binary()) + .args(["--json", "ls", "test/bucket", "--versions"]) + .env("RC_CONFIG_DIR", config_dir.path()) + .env( + "RC_HOST_test", + format!( + "http://accesskey:secretkey@{}", + endpoint.trim_start_matches("http://") + ), + ) + .output() + .expect("run rc command"); assert_eq!( output.status.code(), Some(3), diff --git a/crates/cli/tests/snapshots/golden__alias_tests__alias_list_with_aliases.snap b/crates/cli/tests/snapshots/golden__alias_tests__alias_list_with_aliases.snap index 83fd3a7..e47ac6e 100644 --- a/crates/cli/tests/snapshots/golden__alias_tests__alias_list_with_aliases.snap +++ b/crates/cli/tests/snapshots/golden__alias_tests__alias_list_with_aliases.snap @@ -6,7 +6,7 @@ expression: json { "aliases": [ { - "auth_mode": "sigv4", + "auth_mode": "anonymous", "bucket_lookup": "auto", "endpoint": "http://localhost:9000", "mtls": false, @@ -14,7 +14,7 @@ expression: json "region": "us-east-1" }, { - "auth_mode": "sigv4", + "auth_mode": "anonymous", "bucket_lookup": "auto", "endpoint": "https://s3.amazonaws.com", "mtls": false,