diff --git a/src/scitokens.cpp b/src/scitokens.cpp index 1d53f16..2693140 100644 --- a/src/scitokens.cpp +++ b/src/scitokens.cpp @@ -1,5 +1,10 @@ +#include +#include #include +#include +#include #include +#include #include #include @@ -10,15 +15,89 @@ * GLOBALS */ -// Cache timeout config -std::atomic_int configurer::Configuration::m_next_update_delta{600}; -std::atomic_int configurer::Configuration::m_expiry_delta{4 * 24 * 3600}; +// These are kept for backwards compatibility but are now handled by +// construct-on-first-use in the Configuration class accessor functions +// See scitokens_internal.h for the new implementation +std::atomic_int configurer::Configuration::m_next_update_delta{0}; +std::atomic_int configurer::Configuration::m_expiry_delta{0}; +std::shared_ptr configurer::Configuration::m_cache_home; +std::shared_ptr configurer::Configuration::m_tls_ca_file; -// SciTokens cache home config -std::shared_ptr configurer::Configuration::m_cache_home = - std::make_shared(""); -std::shared_ptr configurer::Configuration::m_tls_ca_file = - std::make_shared(""); +namespace { + +// Helper function to convert string to lowercase +std::string to_lowercase(const std::string &str) { + std::string result = str; + std::transform(result.begin(), result.end(), result.begin(), + [](unsigned char c) { return std::tolower(c); }); + return result; +} + +// Load configuration from environment variables on library initialization +void load_config_from_environment() { + // List of known configuration keys with their types and corresponding env + // var names + struct ConfigMapping { + const char *config_key; + const char *env_var_suffix; // After SCITOKEN_CONFIG_ + bool is_int; + }; + + const std::array known_configs = { + {{"keycache.update_interval_s", "KEYCACHE_UPDATE_INTERVAL_S", true}, + {"keycache.expiration_interval_s", "KEYCACHE_EXPIRATION_INTERVAL_S", + true}, + {"keycache.cache_home", "KEYCACHE_CACHE_HOME", false}, + {"tls.ca_file", "TLS_CA_FILE", false}, + {"monitoring.file", "MONITORING_FILE", false}, + {"monitoring.file_interval_s", "MONITORING_FILE_INTERVAL_S", true}}}; + + const char *prefix = "SCITOKEN_CONFIG_"; + + // Check each known configuration + for (const auto &config : known_configs) { + // Build the full environment variable name + std::string env_var = prefix + std::string(config.env_var_suffix); + + // Also try case variations (uppercase, lowercase, mixed) + const char *env_value = std::getenv(env_var.c_str()); + if (!env_value) { + // Try with lowercase + std::string env_var_lower = to_lowercase(env_var); + env_value = std::getenv(env_var_lower.c_str()); + } + + if (!env_value) { + continue; // Not set in environment + } + + char *err_msg = nullptr; + if (config.is_int) { + try { + int value = std::stoi(env_value); + scitoken_config_set_int(config.config_key, value, &err_msg); + } catch (const std::invalid_argument &) { + // Silently ignore invalid integer format during initialization + } catch (const std::out_of_range &) { + // Silently ignore out-of-range values during initialization + } + } else { + scitoken_config_set_str(config.config_key, env_value, &err_msg); + } + + // Free error message if any (we ignore errors during initialization) + if (err_msg) { + free(err_msg); + } + } +} + +// Use constructor attribute to run on library load +__attribute__((constructor)) void init_scitokens_config() { + load_config_from_environment(); +} + +} // anonymous namespace // Monitoring file config (empty string means disabled) // Protected by mutex; atomic flag for fast-path check diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index 15a74a4..4938f1d 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -1171,7 +1171,9 @@ configurer::Configuration::set_cache_home(const std::string dir_path) { // If setting to "", then we should treat as though it is unsetting the // config if (dir_path.length() == 0) { // User is configuring to empty string - m_cache_home = std::make_shared(dir_path); + std::lock_guard lock(get_cache_home_mutex()); + get_cache_home_string() = dir_path; + get_cache_home_set().store(false, std::memory_order_relaxed); return std::make_pair(true, ""); } @@ -1194,20 +1196,38 @@ configurer::Configuration::set_cache_home(const std::string dir_path) { // Now it exists and we can write to it, set the value and let // scitokens_cache handle the rest - m_cache_home = std::make_shared(cleaned_dir_path); + { + std::lock_guard lock(get_cache_home_mutex()); + get_cache_home_string() = cleaned_dir_path; + get_cache_home_set().store(true, std::memory_order_relaxed); + } return std::make_pair(true, ""); } void configurer::Configuration::set_tls_ca_file(const std::string ca_file) { - m_tls_ca_file = std::make_shared(ca_file); + std::lock_guard lock(get_tls_ca_file_mutex()); + get_tls_ca_file_string() = ca_file; + get_tls_ca_file_set().store(!ca_file.empty(), std::memory_order_relaxed); } std::string configurer::Configuration::get_cache_home() { - return *m_cache_home; + // Fast path: check if the value has been set + if (!get_cache_home_set().load(std::memory_order_relaxed)) { + return ""; + } + // Slow path: acquire lock and read the value + std::lock_guard lock(get_cache_home_mutex()); + return get_cache_home_string(); } std::string configurer::Configuration::get_tls_ca_file() { - return *m_tls_ca_file; + // Fast path: check if the value has been set + if (!get_tls_ca_file_set().load(std::memory_order_relaxed)) { + return ""; + } + // Slow path: acquire lock and read the value + std::lock_guard lock(get_tls_ca_file_mutex()); + return get_tls_ca_file_string(); } // bool configurer::Configuration::check_dir(const std::string dir_path) { diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index 44bf2a7..d8b8970 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -37,13 +37,13 @@ class Configuration { public: Configuration() {} static void set_next_update_delta(int _next_update_delta) { - m_next_update_delta = _next_update_delta; + get_next_update_delta_ref() = _next_update_delta; } - static int get_next_update_delta() { return m_next_update_delta; } + static int get_next_update_delta() { return get_next_update_delta_ref(); } static void set_expiry_delta(int _expiry_delta) { - m_expiry_delta = _expiry_delta; + get_expiry_delta_ref() = _expiry_delta; } - static int get_expiry_delta() { return m_expiry_delta; } + static int get_expiry_delta() { return get_expiry_delta_ref(); } static std::pair set_cache_home(const std::string cache_home); static std::string get_cache_home(); @@ -61,6 +61,45 @@ class Configuration { } private: + // Accessor functions for construct-on-first-use idiom + static std::atomic_int &get_next_update_delta_ref() { + static std::atomic_int instance{600}; + return instance; + } + static std::atomic_int &get_expiry_delta_ref() { + static std::atomic_int instance{4 * 24 * 3600}; + return instance; + } + + // Thread-safe accessors for string configurations + static std::mutex &get_cache_home_mutex() { + static std::mutex instance; + return instance; + } + static std::string &get_cache_home_string() { + static std::string instance; + return instance; + } + static std::atomic &get_cache_home_set() { + static std::atomic instance{false}; + return instance; + } + + static std::mutex &get_tls_ca_file_mutex() { + static std::mutex instance; + return instance; + } + static std::string &get_tls_ca_file_string() { + static std::string instance; + return instance; + } + static std::atomic &get_tls_ca_file_set() { + static std::atomic instance{false}; + return instance; + } + + // Keep old declarations for backwards compatibility (will forward to + // accessor functions) static std::atomic_int m_next_update_delta; static std::atomic_int m_expiry_delta; static std::shared_ptr m_cache_home; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index bdd31e7..6f1f9a4 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -20,6 +20,22 @@ add_test( ${CMAKE_CURRENT_BINARY_DIR}/scitokens-gtest ) +# Environment variable configuration test executable +add_executable(scitokens-env-test test_env_config.cpp) +target_link_libraries(scitokens-env-test SciTokens) + +# Environment variable configuration test +add_test( + NAME + env_config + COMMAND + ${CMAKE_CURRENT_BINARY_DIR}/scitokens-env-test +) + +set_tests_properties(env_config + PROPERTIES + ENVIRONMENT "SCITOKEN_CONFIG_KEYCACHE_UPDATE_INTERVAL_S=1234;SCITOKEN_CONFIG_KEYCACHE_EXPIRATION_INTERVAL_S=5678;SCITOKEN_CONFIG_KEYCACHE_CACHE_HOME=/tmp/test_scitoken_cache;SCITOKEN_CONFIG_TLS_CA_FILE=/tmp/test_ca.pem" +) # Monitoring unit tests add_executable(scitokens-monitoring-test monitoring_test.cpp) if( NOT SCITOKENS_EXTERNAL_GTEST ) diff --git a/test/main.cpp b/test/main.cpp index 0852261..1e1a54f 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -943,6 +943,91 @@ TEST_F(IssuerSecurityTest, SpecialCharacterIssuer) { EXPECT_NE(error_message.find("\""), std::string::npos); } +// Test suite for environment variable configuration +class EnvConfigTest : public ::testing::Test { + protected: + void SetUp() override { + // Save original config values + char *err_msg = nullptr; + original_update_interval = + scitoken_config_get_int("keycache.update_interval_s", &err_msg); + original_expiry_interval = + scitoken_config_get_int("keycache.expiration_interval_s", &err_msg); + + char *cache_home = nullptr; + scitoken_config_get_str("keycache.cache_home", &cache_home, &err_msg); + if (cache_home) { + original_cache_home = cache_home; + free(cache_home); + } + + char *ca_file = nullptr; + scitoken_config_get_str("tls.ca_file", &ca_file, &err_msg); + if (ca_file) { + original_ca_file = ca_file; + free(ca_file); + } + } + + void TearDown() override { + // Restore original config values + char *err_msg = nullptr; + scitoken_config_set_int("keycache.update_interval_s", + original_update_interval, &err_msg); + scitoken_config_set_int("keycache.expiration_interval_s", + original_expiry_interval, &err_msg); + scitoken_config_set_str("keycache.cache_home", + original_cache_home.c_str(), &err_msg); + scitoken_config_set_str("tls.ca_file", original_ca_file.c_str(), + &err_msg); + } + + int original_update_interval = 600; + int original_expiry_interval = 4 * 24 * 3600; + std::string original_cache_home; + std::string original_ca_file; +}; + +TEST_F(EnvConfigTest, IntConfigFromEnv) { + // Note: This test verifies that the environment variable was read at + // library load time We can't test setting environment variables after + // library load in the same process This test would need to be run with + // environment variables set before starting the test + + // Test that we can manually set and get config values + char *err_msg = nullptr; + int test_value = 1234; + auto rv = scitoken_config_set_int("keycache.update_interval_s", test_value, + &err_msg); + ASSERT_EQ(rv, 0) << (err_msg ? err_msg : ""); + + int retrieved = + scitoken_config_get_int("keycache.update_interval_s", &err_msg); + EXPECT_EQ(retrieved, test_value) << (err_msg ? err_msg : ""); + + if (err_msg) + free(err_msg); +} + +TEST_F(EnvConfigTest, StringConfigFromEnv) { + // Test that we can manually set and get string config values + char *err_msg = nullptr; + const char *test_path = "/tmp/test_cache"; + auto rv = + scitoken_config_set_str("keycache.cache_home", test_path, &err_msg); + ASSERT_EQ(rv, 0) << (err_msg ? err_msg : ""); + + char *output = nullptr; + rv = scitoken_config_get_str("keycache.cache_home", &output, &err_msg); + ASSERT_EQ(rv, 0) << (err_msg ? err_msg : ""); + ASSERT_TRUE(output != nullptr); + EXPECT_STREQ(output, test_path); + + free(output); + if (err_msg) + free(err_msg); +} + int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); diff --git a/test/test_env_config.cpp b/test/test_env_config.cpp new file mode 100644 index 0000000..0acef6c --- /dev/null +++ b/test/test_env_config.cpp @@ -0,0 +1,162 @@ +/** + * Test program to verify environment variable configuration loading. + * This must be run as a separate process with environment variables set + * before the library is loaded to properly test the constructor function. + */ + +#include "scitokens.h" +#include +#include +#include +#include + +int main() { + int failures = 0; + char *err_msg = nullptr; + + // Test 1: Check if SCITOKEN_CONFIG_KEYCACHE_UPDATE_INTERVAL_S was loaded + const char *env_update = + std::getenv("SCITOKEN_CONFIG_KEYCACHE_UPDATE_INTERVAL_S"); + if (env_update) { + try { + int expected = std::stoi(env_update); + int actual = + scitoken_config_get_int("keycache.update_interval_s", &err_msg); + if (actual != expected) { + std::cerr << "FAIL: keycache.update_interval_s expected " + << expected << " but got " << actual << std::endl; + if (err_msg) { + std::cerr << "Error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + failures++; + } else { + std::cout << "PASS: keycache.update_interval_s = " << actual + << std::endl; + } + } catch (const std::exception &e) { + std::cerr << "FAIL: Could not parse env var value: " << e.what() + << std::endl; + failures++; + } + } + + // Test 2: Check if SCITOKEN_CONFIG_KEYCACHE_EXPIRATION_INTERVAL_S was + // loaded + const char *env_expiry = + std::getenv("SCITOKEN_CONFIG_KEYCACHE_EXPIRATION_INTERVAL_S"); + if (env_expiry) { + try { + int expected = std::stoi(env_expiry); + int actual = scitoken_config_get_int( + "keycache.expiration_interval_s", &err_msg); + if (actual != expected) { + std::cerr << "FAIL: keycache.expiration_interval_s expected " + << expected << " but got " << actual << std::endl; + if (err_msg) { + std::cerr << "Error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + failures++; + } else { + std::cout << "PASS: keycache.expiration_interval_s = " << actual + << std::endl; + } + } catch (const std::exception &e) { + std::cerr << "FAIL: Could not parse env var value: " << e.what() + << std::endl; + failures++; + } + } + + // Test 3: Check if SCITOKEN_CONFIG_KEYCACHE_CACHE_HOME was loaded + const char *env_cache = std::getenv("SCITOKEN_CONFIG_KEYCACHE_CACHE_HOME"); + if (env_cache) { + char *actual = nullptr; + int rv = + scitoken_config_get_str("keycache.cache_home", &actual, &err_msg); + if (rv != 0 || !actual) { + std::cerr << "FAIL: Could not retrieve keycache.cache_home" + << std::endl; + if (err_msg) { + std::cerr << "Error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + failures++; + } else if (strcmp(actual, env_cache) != 0) { + std::cerr << "FAIL: keycache.cache_home expected '" << env_cache + << "' but got '" << actual << "'" << std::endl; + failures++; + } else { + std::cout << "PASS: keycache.cache_home = " << actual << std::endl; + } + if (actual) + free(actual); + } + + // Test 4: Check if SCITOKEN_CONFIG_TLS_CA_FILE was loaded + const char *env_ca = std::getenv("SCITOKEN_CONFIG_TLS_CA_FILE"); + if (env_ca) { + char *actual = nullptr; + int rv = scitoken_config_get_str("tls.ca_file", &actual, &err_msg); + if (rv != 0 || !actual) { + std::cerr << "FAIL: Could not retrieve tls.ca_file" << std::endl; + if (err_msg) { + std::cerr << "Error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + failures++; + } else if (strcmp(actual, env_ca) != 0) { + std::cerr << "FAIL: tls.ca_file expected '" << env_ca + << "' but got '" << actual << "'" << std::endl; + failures++; + } else { + std::cout << "PASS: tls.ca_file = " << actual << std::endl; + } + if (actual) + free(actual); + } + + // Test 5: Test case insensitivity (lowercase env var) + const char *env_lower = + std::getenv("scitoken_config_keycache_update_interval_s"); + if (env_lower) { + try { + int expected = std::stoi(env_lower); + int actual = + scitoken_config_get_int("keycache.update_interval_s", &err_msg); + if (actual != expected) { + std::cerr << "FAIL: lowercase env var - " + "keycache.update_interval_s expected " + << expected << " but got " << actual << std::endl; + if (err_msg) { + std::cerr << "Error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + failures++; + } else { + std::cout + << "PASS: lowercase env var - keycache.update_interval_s = " + << actual << std::endl; + } + } catch (const std::exception &e) { + std::cerr << "FAIL: Could not parse env var value: " << e.what() + << std::endl; + failures++; + } + } + + if (failures == 0) { + std::cout << "\nAll environment variable configuration tests passed!" + << std::endl; + return 0; + } else { + std::cerr << "\n" << failures << " test(s) failed!" << std::endl; + return 1; + } +}