Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 87 additions & 8 deletions src/scitokens.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#include <algorithm>
#include <array>
#include <atomic>
#include <cctype>
#include <cstdlib>
#include <exception>
#include <stdexcept>
#include <string.h>
#include <sys/stat.h>

Expand All @@ -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<std::string> configurer::Configuration::m_cache_home;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::shared_ptr is not thread-safe; there may be multiple writers to these strings.

Instead, replace with a mutex around a std::string.

To keep a "fast path", add an atomic flag that indicates the variable may have been set. Updates should be atomic but we can use relaxed semantics as to when they become available. If the flag is set, then the mutex should be taken and the variables set. If the flag is not set, then assume the variables are unset.

@copilot - please make this small adjustment. Run clang-format once done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 87b727b. Replaced std::shared_ptr<std::string> with mutex-protected std::string and added atomic flags for fast-path checking with relaxed memory ordering. The get functions now check the atomic flag first (fast path) and only acquire the mutex if the value has been set (slow path).

std::shared_ptr<std::string> configurer::Configuration::m_tls_ca_file;

// SciTokens cache home config
std::shared_ptr<std::string> configurer::Configuration::m_cache_home =
std::make_shared<std::string>("");
std::shared_ptr<std::string> configurer::Configuration::m_tls_ca_file =
std::make_shared<std::string>("");
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<ConfigMapping, 6> 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
Expand Down
30 changes: 25 additions & 5 deletions src/scitokens_internal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>(dir_path);
std::lock_guard<std::mutex> 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, "");
}

Expand All @@ -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<std::string>(cleaned_dir_path);
{
std::lock_guard<std::mutex> 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<std::string>(ca_file);
std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> lock(get_tls_ca_file_mutex());
return get_tls_ca_file_string();
}

// bool configurer::Configuration::check_dir(const std::string dir_path) {
Expand Down
47 changes: 43 additions & 4 deletions src/scitokens_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<bool, std::string>
set_cache_home(const std::string cache_home);
static std::string get_cache_home();
Expand All @@ -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<bool> &get_cache_home_set() {
static std::atomic<bool> 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<bool> &get_tls_ca_file_set() {
static std::atomic<bool> 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<std::string> m_cache_home;
Expand Down
16 changes: 16 additions & 0 deletions test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
Expand Down
85 changes: 85 additions & 0 deletions test/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading