diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8bbbc62..07bc72f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,30 @@ jobs: - name: Run tests run: cargo test --workspace --verbose + # User Story 2b: Run feature-gated integration test using mock-keyring + # This job runs the integration test that depends on the `mock-keyring` feature. + mock-keyring-test: + name: Test (mock-keyring integration) + runs-on: ubuntu-latest + strategy: + matrix: + rust: [stable] + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Install system dependencies + run: make deps + + - name: Setup Rust toolchain + uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + toolchain: ${{ matrix.rust }} + + - name: Run mock-keyring integration test + # Run only the integration test that is gated by the `mock-keyring` feature + run: cargo test -p akon-core --test integration_keyring_tests --features mock-keyring -- --nocapture + # User Story 3: Build Verification # Verifies successful compilation in release mode build: diff --git a/Cargo.toml b/Cargo.toml index 56c0db7..c0004bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ resolver = "2" [package] name = "akon" -version = "1.2.1" +version = "1.2.2" edition = "2021" authors = ["vcwild"] description = "A CLI tool for managing VPN connections with OpenConnect" diff --git a/README.md b/README.md index ca03cd3..d02d92b 100644 --- a/README.md +++ b/README.md @@ -225,12 +225,12 @@ protocol = "f5" health_check_endpoint = "https://your-internal-server.example.com/" # Optional: Customize retry behavior (defaults shown) -max_attempts = 5 # Maximum reconnection attempts +max_attempts = 3 # Maximum reconnection attempts (default) base_interval_secs = 5 # Initial retry delay backoff_multiplier = 2 # Exponential backoff multiplier max_interval_secs = 60 # Maximum delay between attempts -consecutive_failures_threshold = 2 # Health check failures before reconnection -health_check_interval_secs = 60 # How often to check health +consecutive_failures_threshold = 1 # Health check failures before reconnection (default) +health_check_interval_secs = 10 # How often to check health (default) ``` ## Why "akon"? @@ -382,6 +382,35 @@ cargo tarpaulin --out Html # View coverage report open tarpaulin-report.html + +## Testing and mock keyring + +For tests that need a keyring implementation (CI or local), akon-core provides a lightweight +"mock keyring" implementation which stores credentials in-memory. This is useful for unit and +integration tests that must not interact with the system keyring. + +The mock keyring and its test-only dependency (`lazy_static`) are behind a feature flag +so they are opt-in for consumers of `akon-core`: + +- Feature name: `mock-keyring` +- Optional dependency: `lazy_static` (enabled only when `mock-keyring` is enabled) + +Run tests that require the mock keyring with: + +```bash +# Run a single integration test using the mock keyring +cargo test -p akon-core --test integration_keyring_tests --features mock-keyring -- --nocapture +``` + +Notes: + +- `lazy_static` is declared as an optional dependency enabled by `mock-keyring` and also present + as a `dev-dependency` so developers can run tests locally without enabling the feature. +- This means the `lazy_static` crate is not linked into production binaries unless a consumer + enables `mock-keyring` explicitly. +- The mock keyring mirrors production retrieval behavior for PINs (the runtime truncates + retrieved PINs to 30 characters). Tests validate truncation and password assembly. + ``` ## Contributing diff --git a/akon-core/Cargo.toml b/akon-core/Cargo.toml index 2921976..e1e34c2 100644 --- a/akon-core/Cargo.toml +++ b/akon-core/Cargo.toml @@ -1,11 +1,12 @@ [package] edition = "2021" name = "akon-core" -version = "1.2.1" +version = "1.2.2" [features] default = [] -mock-keyring = [] +# Enable the mock keyring implementation and its test-only dependencies +mock-keyring = ["lazy_static"] [lints.rust] dead_code = "deny" @@ -29,6 +30,8 @@ data-encoding = "2.9.0" sha1 = "0.10.6" regex = "1.10" chrono = "0.4" +# lazy_static is optional and enabled via the `mock-keyring` feature +lazy_static = { version = "1.5", optional = true } # Network interruption detection dependencies zbus = "4.0" diff --git a/akon-core/src/auth/keyring.rs b/akon-core/src/auth/keyring.rs index 07fd4f1..7581d09 100644 --- a/akon-core/src/auth/keyring.rs +++ b/akon-core/src/auth/keyring.rs @@ -4,15 +4,12 @@ //! sensitive VPN credentials securely. use crate::error::{AkonError, KeyringError}; -use crate::types::{Pin, KEYRING_SERVICE_PIN}; +use crate::types::{Pin, KEYRING_SERVICE_OTP, KEYRING_SERVICE_PIN}; use keyring::Entry; -/// Service name used for storing credentials in the keyring (legacy) -const SERVICE_NAME: &str = "akon-vpn"; - /// Store an OTP secret in the system keyring pub fn store_otp_secret(username: &str, secret: &str) -> Result<(), AkonError> { - let entry = Entry::new(SERVICE_NAME, username) + let entry = Entry::new(KEYRING_SERVICE_OTP, username) .map_err(|_| AkonError::Keyring(KeyringError::ServiceUnavailable))?; entry @@ -24,7 +21,7 @@ pub fn store_otp_secret(username: &str, secret: &str) -> Result<(), AkonError> { /// Retrieve an OTP secret from the system keyring pub fn retrieve_otp_secret(username: &str) -> Result { - let entry = Entry::new(SERVICE_NAME, username) + let entry = Entry::new(KEYRING_SERVICE_OTP, username) .map_err(|_| AkonError::Keyring(KeyringError::ServiceUnavailable))?; entry @@ -34,7 +31,7 @@ pub fn retrieve_otp_secret(username: &str) -> Result { /// Check if an OTP secret exists in the keyring for the given username pub fn has_otp_secret(username: &str) -> Result { - let entry = Entry::new(SERVICE_NAME, username) + let entry = Entry::new(KEYRING_SERVICE_OTP, username) .map_err(|_| AkonError::Keyring(KeyringError::ServiceUnavailable))?; match entry.get_password() { diff --git a/akon-core/src/auth/keyring_mock.rs b/akon-core/src/auth/keyring_mock.rs index 543465d..95e4543 100644 --- a/akon-core/src/auth/keyring_mock.rs +++ b/akon-core/src/auth/keyring_mock.rs @@ -4,7 +4,7 @@ //! system keyring access. Used in CI environments and for testing. use crate::error::{AkonError, KeyringError}; -use crate::types::Pin; +use crate::types::{Pin, KEYRING_SERVICE_OTP, KEYRING_SERVICE_PIN}; use std::collections::HashMap; use std::sync::Mutex; @@ -17,15 +17,9 @@ fn make_key(service: &str, username: &str) -> String { format!("{}:{}", service, username) } -/// Service name used for storing credentials in the keyring (legacy) -const SERVICE_NAME: &str = "akon-vpn"; - -/// Service name for PIN storage -const SERVICE_NAME_PIN: &str = "akon-vpn-pin"; - /// Store an OTP secret in the mock keyring pub fn store_otp_secret(username: &str, secret: &str) -> Result<(), AkonError> { - let key = make_key(SERVICE_NAME, username); + let key = make_key(KEYRING_SERVICE_OTP, username); let mut keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::StoreFailed))?; @@ -35,7 +29,7 @@ pub fn store_otp_secret(username: &str, secret: &str) -> Result<(), AkonError> { /// Retrieve an OTP secret from the mock keyring pub fn retrieve_otp_secret(username: &str) -> Result { - let key = make_key(SERVICE_NAME, username); + let key = make_key(KEYRING_SERVICE_OTP, username); let keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::RetrieveFailed))?; @@ -47,7 +41,7 @@ pub fn retrieve_otp_secret(username: &str) -> Result { /// Check if an OTP secret exists in the mock keyring for the given username pub fn has_otp_secret(username: &str) -> Result { - let key = make_key(SERVICE_NAME, username); + let key = make_key(KEYRING_SERVICE_OTP, username); let keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::ServiceUnavailable))?; @@ -56,7 +50,7 @@ pub fn has_otp_secret(username: &str) -> Result { /// Delete an OTP secret from the mock keyring pub fn delete_otp_secret(username: &str) -> Result<(), AkonError> { - let key = make_key(SERVICE_NAME, username); + let key = make_key(KEYRING_SERVICE_OTP, username); let mut keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::StoreFailed))?; @@ -66,7 +60,7 @@ pub fn delete_otp_secret(username: &str) -> Result<(), AkonError> { /// Store a PIN in the mock keyring pub fn store_pin(username: &str, pin: &Pin) -> Result<(), AkonError> { - let key = make_key(SERVICE_NAME_PIN, username); + let key = make_key(KEYRING_SERVICE_PIN, username); let mut keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::StoreFailed))?; @@ -76,7 +70,7 @@ pub fn store_pin(username: &str, pin: &Pin) -> Result<(), AkonError> { /// Retrieve a PIN from the mock keyring pub fn retrieve_pin(username: &str) -> Result { - let key = make_key(SERVICE_NAME_PIN, username); + let key = make_key(KEYRING_SERVICE_PIN, username); let keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::PinNotFound))?; @@ -84,12 +78,20 @@ pub fn retrieve_pin(username: &str) -> Result { .get(&key) .cloned() .ok_or(AkonError::Keyring(KeyringError::PinNotFound))?; - Pin::new(pin_str).map_err(AkonError::Otp) + // Mirror production retrieval behavior: enforce a 30-char internal limit + let pin_trimmed = pin_str.trim().to_string(); + let stored = if pin_trimmed.chars().count() > 30 { + pin_trimmed.chars().take(30).collect::() + } else { + pin_trimmed.clone() + }; + + Ok(Pin::from_unchecked(stored)) } /// Check if a PIN exists in the mock keyring for the given username pub fn has_pin(username: &str) -> Result { - let key = make_key(SERVICE_NAME_PIN, username); + let key = make_key(KEYRING_SERVICE_PIN, username); let keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::ServiceUnavailable))?; @@ -98,7 +100,7 @@ pub fn has_pin(username: &str) -> Result { /// Delete a PIN from the mock keyring pub fn delete_pin(username: &str) -> Result<(), AkonError> { - let key = make_key(SERVICE_NAME_PIN, username); + let key = make_key(KEYRING_SERVICE_PIN, username); let mut keyring = MOCK_KEYRING .lock() .map_err(|_| AkonError::Keyring(KeyringError::StoreFailed))?; @@ -151,4 +153,52 @@ mod tests { delete_pin(username).expect("Failed to delete PIN"); assert!(!has_pin(username).expect("Failed to check PIN after delete")); } + + #[test] + fn test_long_pin_truncation_and_generate_password() { + use crate::auth::password::generate_password; + + let username = "test_long_pin_user"; + + // Clean up first + let _ = delete_pin(username); + let _ = delete_otp_secret(username); + + // Create a long PIN (>30 chars) + let long_pin = "012345678901234567890123456789012345".to_string(); // 36 chars + let pin = Pin::from_unchecked(long_pin.clone()); + + // Store long PIN and a valid OTP secret + store_pin(username, &pin).expect("Failed to store long PIN"); + store_otp_secret(username, "JBSWY3DPEHPK3PXP").expect("Failed to store OTP secret"); + + // Now generate password using generate_password which should retrieve and truncate + let result = generate_password(username); + assert!( + result.is_ok(), + "generate_password failed: {:?}", + result.err() + ); + + let password = result.unwrap(); + let pwd_str = password.expose(); + + // The stored PIN should be silently truncated to 30 chars + let expected_pin_prefix = long_pin.chars().take(30).collect::(); + assert!( + pwd_str.starts_with(&expected_pin_prefix), + "Password does not start with truncated PIN: {} vs {}", + pwd_str, + expected_pin_prefix + ); + + // OTP part should be 6 digits at the end + assert!(pwd_str.len() >= 6); + let otp_part = &pwd_str[pwd_str.len() - 6..]; + assert!(otp_part.chars().all(|c| c.is_ascii_digit())); + + // Clean up + delete_pin(username).expect("Failed to delete PIN"); + delete_otp_secret(username).expect("Failed to delete OTP"); + } } diff --git a/akon-core/src/vpn/reconnection.rs b/akon-core/src/vpn/reconnection.rs index 788c0c2..3e4d116 100644 --- a/akon-core/src/vpn/reconnection.rs +++ b/akon-core/src/vpn/reconnection.rs @@ -39,7 +39,7 @@ pub struct ReconnectionPolicy { } fn default_max_attempts() -> u32 { - 5 + 3 } fn default_base_interval() -> u32 { 5 @@ -51,10 +51,10 @@ fn default_max_interval() -> u32 { 60 } fn default_consecutive_failures() -> u32 { - 3 + 1 } fn default_health_check_interval() -> u64 { - 60 + 10 } impl ReconnectionPolicy { diff --git a/akon-core/tests/config_tests.rs b/akon-core/tests/config_tests.rs index c395e56..cc4bb9a 100644 --- a/akon-core/tests/config_tests.rs +++ b/akon-core/tests/config_tests.rs @@ -103,12 +103,12 @@ mod reconnection_policy_tests { let policy: ReconnectionPolicy = toml::from_str(toml_str).unwrap(); // Check defaults are applied - assert_eq!(policy.max_attempts, 5); // default + assert_eq!(policy.max_attempts, 3); // default (updated) assert_eq!(policy.base_interval_secs, 5); // default assert_eq!(policy.backoff_multiplier, 2); // default assert_eq!(policy.max_interval_secs, 60); // default - assert_eq!(policy.consecutive_failures_threshold, 3); // default - assert_eq!(policy.health_check_interval_secs, 60); // default + assert_eq!(policy.consecutive_failures_threshold, 1); // default (updated) + assert_eq!(policy.health_check_interval_secs, 10); // default (updated) assert_eq!( policy.health_check_endpoint, "https://vpn.example.com/health" diff --git a/akon-core/tests/fixtures/test_config.toml b/akon-core/tests/fixtures/test_config.toml index 090d89b..a4b73f0 100644 --- a/akon-core/tests/fixtures/test_config.toml +++ b/akon-core/tests/fixtures/test_config.toml @@ -7,8 +7,8 @@ username = "testuser" [reconnection] backoff_multiplier = 2 base_interval_secs = 5 -consecutive_failures_threshold = 3 +consecutive_failures_threshold = 1 health_check_endpoint = "https://vpn.example.com/healthz" -health_check_interval_secs = 60 -max_attempts = 5 +health_check_interval_secs = 10 +max_attempts = 3 max_interval_secs = 60 diff --git a/akon-core/tests/integration_keyring_tests.rs b/akon-core/tests/integration_keyring_tests.rs new file mode 100644 index 0000000..e3b8944 --- /dev/null +++ b/akon-core/tests/integration_keyring_tests.rs @@ -0,0 +1,45 @@ +// This integration test requires the mock keyring to be enabled via the +// `mock-keyring` feature. Run with: +// +// cargo test -p akon-core --test integration_keyring_tests --features mock-keyring +// +#[cfg(feature = "mock-keyring")] +#[test] +fn integration_long_pin_truncation() { + use akon_core::auth::keyring; + use akon_core::types::Pin; + let username = "integration_long_pin_user"; + + // Clean up any existing entries + let _ = keyring::delete_pin(username); + let _ = keyring::delete_otp_secret(username); + + // Create and store a long PIN (>30 chars) + let long_pin = "abcdefghijklmnopqrstuvwxyz0123456789".to_string(); // 36 chars + let pin = Pin::from_unchecked(long_pin.clone()); + keyring::store_pin(username, &pin).expect("Failed to store long PIN"); + + // Store a valid OTP secret + let otp = "JBSWY3DPEHPK3PXP"; // valid base32 + keyring::store_otp_secret(username, otp).expect("Failed to store OTP secret"); + + // Generate password + let res = akon_core::auth::password::generate_password(username); + assert!(res.is_ok(), "generate_password failed: {:?}", res.err()); + + let password = res.unwrap(); + let pwd = password.expose(); + + // Expect the password to start with the first 30 chars of the stored PIN + let expected_prefix: String = long_pin.chars().take(30).collect(); + assert!( + pwd.starts_with(&expected_prefix), + "Password prefix mismatch: {} vs {}", + pwd, + expected_prefix + ); + + // Clean up + let _ = keyring::delete_pin(username); + let _ = keyring::delete_otp_secret(username); +} diff --git a/akon-core/tests/reconnection_config_tests.rs b/akon-core/tests/reconnection_config_tests.rs index fc5df78..354181a 100644 --- a/akon-core/tests/reconnection_config_tests.rs +++ b/akon-core/tests/reconnection_config_tests.rs @@ -15,12 +15,12 @@ fn test_parse_reconnection_config_from_file() { let policy = config .reconnection_policy() .expect("Should have reconnection policy"); - assert_eq!(policy.max_attempts, 5); + assert_eq!(policy.max_attempts, 3); assert_eq!(policy.base_interval_secs, 5); assert_eq!(policy.backoff_multiplier, 2); assert_eq!(policy.max_interval_secs, 60); - assert_eq!(policy.consecutive_failures_threshold, 3); - assert_eq!(policy.health_check_interval_secs, 60); + assert_eq!(policy.consecutive_failures_threshold, 1); + assert_eq!(policy.health_check_interval_secs, 10); assert_eq!( policy.health_check_endpoint, "https://vpn.example.com/healthz" @@ -130,7 +130,7 @@ fn test_health_check_endpoint_required() { // Given: A config with reconnection but missing endpoint let config_toml = r#" [reconnection] - max_attempts = 5 + max_attempts = 3 "#; // When: Parsing the config diff --git a/tests/get_password_tests.rs b/tests/get_password_tests.rs index c1caeea..2c9a399 100644 --- a/tests/get_password_tests.rs +++ b/tests/get_password_tests.rs @@ -3,9 +3,10 @@ //! Tests the get-password command behavior including error handling //! and output format validation. -use akon_core::auth::keyring; +use akon_core::types::{KEYRING_SERVICE_OTP, KEYRING_SERVICE_PIN}; use std::env; use std::fs; +use std::io::Write; use std::process::Command; const AKON_BINARY: &str = "target/debug/akon"; @@ -52,13 +53,54 @@ timeout = 30 ) .expect("Failed to write config file"); - // Store test credentials in keyring + // Store test credentials in the system keyring so the spawned binary can read them. + // We do this with `secret-tool` (GNOME keyring). This test is intended for + // local interactive environments and is skipped in CI above. let test_username = "__akon_get_password_test__"; let test_secret = "JBSWY3DPEHPK3PXP"; // Valid base32 - let test_pin = akon_core::types::Pin::new("1234".to_string()).expect("Valid PIN"); + let test_pin_value = "1234"; + + // Helper to store a secret using `secret-tool store` by writing the secret to stdin. + fn store_system_secret(service: &str, username: &str, secret: &str) -> Result<(), String> { + let mut child = Command::new("secret-tool") + .args([ + "store", + "--label", + "akon-test", + "service", + service, + "username", + username, + ]) + .stdin(std::process::Stdio::piped()) + .spawn() + .map_err(|e| format!("failed to spawn secret-tool: {}", e))?; + + if let Some(mut stdin) = child.stdin.take() { + stdin + .write_all(secret.as_bytes()) + .map_err(|e| format!("failed writing to secret-tool stdin: {}", e))?; + } + + let status = child + .wait() + .map_err(|e| format!("failed waiting for secret-tool: {}", e))?; + + if status.success() { + Ok(()) + } else { + Err(format!( + "secret-tool exited with status: {:?}", + status.code() + )) + } + } + + store_system_secret(KEYRING_SERVICE_OTP, test_username, test_secret) + .expect("Failed to store test OTP secret in system keyring"); - keyring::store_otp_secret(test_username, test_secret).expect("Failed to store test OTP secret"); - keyring::store_pin(test_username, &test_pin).expect("Failed to store test PIN"); + store_system_secret(KEYRING_SERVICE_PIN, test_username, test_pin_value) + .expect("Failed to store test PIN in system keyring"); // Run get-password command let output = Command::new(AKON_BINARY) @@ -67,9 +109,25 @@ timeout = 30 .output() .expect("Failed to run get-password"); - // Clean up - let _ = keyring::delete_otp_secret(test_username); - let _ = keyring::delete_pin(test_username); + // Clean up: remove stored secrets from system keyring and restore env + let _ = Command::new("secret-tool") + .args([ + "clear", + "service", + KEYRING_SERVICE_OTP, + "username", + test_username, + ]) + .status(); + let _ = Command::new("secret-tool") + .args([ + "clear", + "service", + KEYRING_SERVICE_PIN, + "username", + test_username, + ]) + .status(); env::remove_var("AKON_CONFIG_DIR"); // Should succeed and output a complete password (PIN + OTP)