From d4e73326eb79966e034ddcfb186a955f34d97ad6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 8 Dec 2025 22:37:12 +0000 Subject: [PATCH 1/6] Add monitoring API infrastructure --- CMakeLists.txt | 9 +- src/scitokens.cpp | 44 +++++- src/scitokens.h | 21 +++ src/scitokens_internal.cpp | 213 ++++++++++++++------------ src/scitokens_internal.h | 208 +++++++++++++++++++++++-- src/scitokens_monitoring.cpp | 146 ++++++++++++++++++ src/test_monitoring.cpp | 50 ++++++ src/test_monitoring_comprehensive.cpp | 105 +++++++++++++ 8 files changed, 679 insertions(+), 117 deletions(-) create mode 100644 src/scitokens_monitoring.cpp create mode 100644 src/test_monitoring.cpp create mode 100644 src/test_monitoring_comprehensive.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 69bf486..117549b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,7 +44,7 @@ pkg_check_modules(SQLITE REQUIRED sqlite3) endif() -add_library(SciTokens SHARED src/scitokens.cpp src/scitokens_internal.cpp src/scitokens_cache.cpp) +add_library(SciTokens SHARED src/scitokens.cpp src/scitokens_internal.cpp src/scitokens_cache.cpp src/scitokens_monitoring.cpp) target_compile_features(SciTokens PUBLIC cxx_std_11) # Use at least C++11 for building and when linking to scitokens target_include_directories(SciTokens PUBLIC ${JWT_CPP_INCLUDES} "${PROJECT_SOURCE_DIR}/src" PRIVATE ${CURL_INCLUDE_DIRS} ${OPENSSL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_DIRS} ${SQLITE_INCLUDE_DIRS} ${UUID_INCLUDE_DIRS}) @@ -75,10 +75,17 @@ target_link_libraries(scitokens-list-access SciTokens) add_executable(scitokens-create src/create.cpp) target_link_libraries(scitokens-create SciTokens) + add_executable(scitokens-generate-jwks src/generate_jwks.cpp) target_include_directories(scitokens-generate-jwks PRIVATE ${OPENSSL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_DIRS}) target_link_libraries(scitokens-generate-jwks ${OPENSSL_LIBRARIES} ${LIBCRYPTO_LIBRARIES}) +add_executable(scitokens-test-monitoring src/test_monitoring.cpp) +target_link_libraries(scitokens-test-monitoring SciTokens) + +add_executable(scitokens-test-monitoring-comprehensive src/test_monitoring_comprehensive.cpp) +target_link_libraries(scitokens-test-monitoring-comprehensive SciTokens) + get_directory_property(TARGETS BUILDSYSTEM_TARGETS) install( TARGETS ${TARGETS} diff --git a/src/scitokens.cpp b/src/scitokens.cpp index a11c84d..711f4c0 100644 --- a/src/scitokens.cpp +++ b/src/scitokens.cpp @@ -246,10 +246,12 @@ int scitoken_get_expiration(const SciToken token, long long *expiry, // Float value - convert to integer (truncate) // Float value - convert to integer using std::floor(). // This ensures expiration is not extended by fractional seconds. - result = static_cast(std::floor(claim_value.get())); + result = + static_cast(std::floor(claim_value.get())); } else { if (err_msg) { - *err_msg = strdup("'exp' claim must be a number (integer or float)"); + *err_msg = + strdup("'exp' claim must be a number (integer or float)"); } return -1; } @@ -1080,9 +1082,9 @@ int scitoken_config_set_str(const char *key, const char *value, return -1; } } else if (_key == "tls.ca_file") { - configurer::Configuration::set_tls_ca_file(value ? std::string(value) : ""); - } - else { + configurer::Configuration::set_tls_ca_file(value ? std::string(value) + : ""); + } else { if (err_msg) { *err_msg = strdup("Key not recognized."); } @@ -1114,3 +1116,35 @@ int scitoken_config_get_str(const char *key, char **output, char **err_msg) { } return 0; } + +int scitoken_get_monitoring_json(char **json_out, char **err_msg) { + if (!json_out) { + if (err_msg) { + *err_msg = strdup("JSON output pointer may not be null."); + } + return -1; + } + try { + std::string json = + scitokens::internal::MonitoringStats::instance().get_json(); + *json_out = strdup(json.c_str()); + } catch (std::exception &exc) { + if (err_msg) { + *err_msg = strdup(exc.what()); + } + return -1; + } + return 0; +} + +int scitoken_reset_monitoring_stats(char **err_msg) { + try { + scitokens::internal::MonitoringStats::instance().reset(); + } catch (std::exception &exc) { + if (err_msg) { + *err_msg = strdup(exc.what()); + } + return -1; + } + return 0; +} diff --git a/src/scitokens.h b/src/scitokens.h index b87003a..4d05202 100644 --- a/src/scitokens.h +++ b/src/scitokens.h @@ -329,6 +329,27 @@ int scitoken_config_set_str(const char *key, const char *value, char **err_msg); */ int scitoken_config_get_str(const char *key, char **output, char **err_msg); +/** + * Get monitoring statistics as a JSON string. + * Returns a JSON object containing per-issuer validation statistics including: + * - successful_validations: count of successful token validations + * - unsuccessful_validations: count of failed token validations + * - expired_tokens: count of expired tokens encountered + * - total_validation_time_s: total validation time in seconds + * - failed_issuer_lookups: count of failed issuer lookups (limited to prevent + * DDoS) + * + * The returned string must be freed by the caller using free(). + * Returns 0 on success, nonzero on failure. + */ +int scitoken_get_monitoring_json(char **json_out, char **err_msg); + +/** + * Reset all monitoring statistics. + * Returns 0 on success, nonzero on failure. + */ +int scitoken_reset_monitoring_stats(char **err_msg); + #ifdef __cplusplus } #endif diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index 91308dd..67116ce 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -652,116 +652,133 @@ SciToken::deserialize_continue(std::unique_ptr status) { std::unique_ptr Validator::get_public_keys_from_web(const std::string &issuer, unsigned timeout) { - std::string openid_metadata, oauth_metadata; - get_metadata_endpoint(issuer, openid_metadata, oauth_metadata); - - std::unique_ptr status(new AsyncStatus()); - status->m_oauth_metadata_url = oauth_metadata; - status->m_cget.reset(new internal::SimpleCurlGet(1024 * 1024, timeout)); - auto cget_status = status->m_cget->perform_start(openid_metadata); - status->m_continue_fetch = true; - if (!cget_status.m_done) { - return status; - } - return get_public_keys_from_web_continue(std::move(status)); + try { + std::string openid_metadata, oauth_metadata; + get_metadata_endpoint(issuer, openid_metadata, oauth_metadata); + + std::unique_ptr status(new AsyncStatus()); + status->m_oauth_metadata_url = oauth_metadata; + status->m_cget.reset(new internal::SimpleCurlGet(1024 * 1024, timeout)); + auto cget_status = status->m_cget->perform_start(openid_metadata); + status->m_continue_fetch = true; + if (!cget_status.m_done) { + return status; + } + return get_public_keys_from_web_continue(std::move(status)); + } catch (const CurlException &e) { + // Rethrow CURL errors during issuer key fetch as IssuerLookupException + throw IssuerLookupException(e.what()); + } } std::unique_ptr Validator::get_public_keys_from_web_continue( std::unique_ptr status) { - char *buffer; - size_t len; + try { + char *buffer; + size_t len; - switch (status->m_state) { + switch (status->m_state) { - case AsyncStatus::DOWNLOAD_METADATA: { - auto cget_status = status->m_cget->perform_continue(); - if (!cget_status.m_done) { - return std::move(status); - } - if (cget_status.m_status_code != 200) { - if (status->m_oauth_fallback) { - throw CurlException("Failed to retrieve metadata provider " - "information for issuer."); - } else { - status->m_oauth_fallback = true; - status->m_cget.reset(new internal::SimpleCurlGet()); - cget_status = - status->m_cget->perform_start(status->m_oauth_metadata_url); - if (!cget_status.m_done) { - return std::move(status); + case AsyncStatus::DOWNLOAD_METADATA: { + auto cget_status = status->m_cget->perform_continue(); + if (!cget_status.m_done) { + return std::move(status); + } + if (cget_status.m_status_code != 200) { + if (status->m_oauth_fallback) { + throw IssuerLookupException( + "Failed to retrieve metadata provider " + "information for issuer."); + } else { + status->m_oauth_fallback = true; + status->m_cget.reset(new internal::SimpleCurlGet()); + cget_status = status->m_cget->perform_start( + status->m_oauth_metadata_url); + if (!cget_status.m_done) { + return std::move(status); + } + return get_public_keys_from_web_continue(std::move(status)); } - return get_public_keys_from_web_continue(std::move(status)); } + status->m_cget->get_data(buffer, len); + std::string metadata(buffer, len); + picojson::value json_obj; + auto err = picojson::parse(json_obj, metadata); + if (!err.empty()) { + throw JsonException("JSON parse failure when downloading from " + "the metadata URL " + + status->m_cget->get_url() + ": " + err); + } + if (!json_obj.is()) { + throw JsonException("Metadata resource " + + status->m_cget->get_url() + + " contains " + "improperly-formatted JSON."); + } + auto top_obj = json_obj.get(); + auto iter = top_obj.find("jwks_uri"); + if (iter == top_obj.end() || (!iter->second.is())) { + throw JsonException("Metadata resource " + + status->m_cget->get_url() + + " is missing 'jwks_uri' string value"); + } + auto jwks_uri = iter->second.get(); + status->m_has_metadata = true; + status->m_state = AsyncStatus::DOWNLOAD_PUBLIC_KEY; + status->m_cget.reset(new internal::SimpleCurlGet()); + status->m_cget->perform_start(jwks_uri); + // This should also fall through the next state } - status->m_cget->get_data(buffer, len); - std::string metadata(buffer, len); - picojson::value json_obj; - auto err = picojson::parse(json_obj, metadata); - if (!err.empty()) { - throw JsonException( - "JSON parse failure when downloading from the metadata URL " + - status->m_cget->get_url() + ": " + err); - } - if (!json_obj.is()) { - throw JsonException("Metadata resource " + - status->m_cget->get_url() + - " contains " - "improperly-formatted JSON."); - } - auto top_obj = json_obj.get(); - auto iter = top_obj.find("jwks_uri"); - if (iter == top_obj.end() || (!iter->second.is())) { - throw JsonException("Metadata resource " + - status->m_cget->get_url() + - " is missing 'jwks_uri' string value"); - } - auto jwks_uri = iter->second.get(); - status->m_has_metadata = true; - status->m_state = AsyncStatus::DOWNLOAD_PUBLIC_KEY; - status->m_cget.reset(new internal::SimpleCurlGet()); - status->m_cget->perform_start(jwks_uri); - // This should also fall through the next state - } - case AsyncStatus::DOWNLOAD_PUBLIC_KEY: { - auto cget_status = status->m_cget->perform_continue(); - if (!cget_status.m_done) { - return std::move(status); - } - if (cget_status.m_status_code != 200) { - throw CurlException("Failed to retrieve the issuer's key set"); - } + case AsyncStatus::DOWNLOAD_PUBLIC_KEY: { + auto cget_status = status->m_cget->perform_continue(); + if (!cget_status.m_done) { + return std::move(status); + } + if (cget_status.m_status_code != 200) { + throw IssuerLookupException( + "Failed to retrieve the issuer's key set"); + } - status->m_cget->get_data(buffer, len); - auto metadata = std::string(buffer, len); - picojson::value json_obj; - auto err = picojson::parse(json_obj, metadata); - if (!err.empty()) { - throw JsonException("JSON parse failure when downloading from the " - " public key URL " + - status->m_cget->get_url() + ": " + err); + status->m_cget->get_data(buffer, len); + auto metadata = std::string(buffer, len); + picojson::value json_obj; + auto err = picojson::parse(json_obj, metadata); + if (!err.empty()) { + throw JsonException( + "JSON parse failure when downloading from the " + " public key URL " + + status->m_cget->get_url() + ": " + err); + } + status->m_cget.reset(); + + auto now = std::time(NULL); + // TODO: take expiration time from the cache-control header in the + // response. + + int next_update_delta = + configurer::Configuration::get_next_update_delta(); + int expiry_delta = configurer::Configuration::get_expiry_delta(); + status->m_next_update = now + next_update_delta; + status->m_expires = now + expiry_delta; + status->m_keys = json_obj; + status->m_continue_fetch = false; + status->m_done = true; + status->m_state = AsyncStatus::DONE; } - status->m_cget.reset(); - - auto now = std::time(NULL); - // TODO: take expiration time from the cache-control header in the - // response. - - int next_update_delta = - configurer::Configuration::get_next_update_delta(); - int expiry_delta = configurer::Configuration::get_expiry_delta(); - status->m_next_update = now + next_update_delta; - status->m_expires = now + expiry_delta; - status->m_keys = json_obj; - status->m_continue_fetch = false; - status->m_done = true; - status->m_state = AsyncStatus::DONE; - } - case AsyncStatus::DONE: - status->m_done = true; - - } // Switch - return std::move(status); + case AsyncStatus::DONE: + status->m_done = true; + + } // Switch + return std::move(status); + } catch (const CurlException &e) { + // Rethrow CURL errors during issuer key fetch as IssuerLookupException + // (unless it's already an IssuerLookupException) + if (dynamic_cast(&e)) { + throw; + } + throw IssuerLookupException(e.what()); + } } std::string Validator::get_jwks(const std::string &issuer) { diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index c947b46..ad81f27 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -1,4 +1,5 @@ +#include #include #include #include @@ -65,6 +66,9 @@ namespace scitokens { namespace internal { +// Forward declaration +class MonitoringStats; + class SimpleCurlGet { int m_maxbytes{1048576}; @@ -110,6 +114,52 @@ class SimpleCurlGet { void *userp); }; +/** + * Statistics for monitoring token validation per issuer. + */ +struct IssuerStats { + std::atomic successful_validations{0}; + std::atomic unsuccessful_validations{0}; + std::atomic expired_tokens{0}; + double total_time_s{0.0}; // Total time in seconds - protected by mutex +}; + +/** + * Monitoring statistics singleton. + * Tracks per-issuer validation statistics and protects against + * resource exhaustion from invalid issuers. + */ +class MonitoringStats { + public: + static MonitoringStats &instance(); + + void record_validation_success(const std::string &issuer, + double duration_s); + void record_validation_failure(const std::string &issuer, + double duration_s); + void record_expired_token(const std::string &issuer); + void record_failed_issuer_lookup(const std::string &issuer); + + std::string get_json() const; + void reset(); + + private: + MonitoringStats() = default; + ~MonitoringStats() = default; + MonitoringStats(const MonitoringStats &) = delete; + MonitoringStats &operator=(const MonitoringStats &) = delete; + + // Limit the number of failed issuer entries to prevent DDoS + static constexpr size_t MAX_FAILED_ISSUERS = 100; + + mutable std::mutex m_mutex; + std::unordered_map m_issuer_stats; + std::unordered_map m_failed_issuer_lookups; + + std::string sanitize_issuer_for_json(const std::string &issuer) const; + void prune_failed_issuers(); +}; + } // namespace internal class UnsupportedKeyException : public std::runtime_error { @@ -129,6 +179,18 @@ class CurlException : public std::runtime_error { explicit CurlException(const std::string &msg) : std::runtime_error(msg) {} }; +class IssuerLookupException : public CurlException { + public: + explicit IssuerLookupException(const std::string &msg) + : CurlException(msg) {} +}; + +class TokenExpiredException : public JWTVerificationException { + public: + explicit TokenExpiredException(const std::string &msg) + : JWTVerificationException(msg) {} +}; + class MissingIssuerException : public std::runtime_error { public: MissingIssuerException() @@ -226,6 +288,8 @@ class AsyncStatus { std::string m_jwt_string; std::string m_public_pem; std::string m_algorithm; + std::chrono::steady_clock::time_point m_start_time; + bool m_monitoring_started{false}; struct timeval get_timeout_val(time_t expiry_time) const { auto now = time(NULL); @@ -407,6 +471,9 @@ class Validator { void set_now(std::chrono::system_clock::time_point now) { m_now = now; } + // Maximum timeout for select() in microseconds for periodic checks + static constexpr long MAX_SELECT_TIMEOUT_US = 50000; // 50ms + std::unique_ptr verify_async(const SciToken &scitoken) { const jwt::decoded_jwt *jwt_decoded = scitoken.m_decoded.get(); @@ -418,24 +485,96 @@ class Validator { } void verify(const SciToken &scitoken, time_t expiry_time) { - auto result = verify_async(scitoken); - while (!result->m_done) { - auto timeout_val = result->get_timeout_val(expiry_time); - select(result->get_max_fd() + 1, result->get_read_fd_set(), - result->get_write_fd_set(), result->get_exc_fd_set(), - &timeout_val); - if (time(NULL) >= expiry_time) { - throw CurlException("Timeout when loading the OIDC metadata."); + std::string issuer = ""; + auto start_time = std::chrono::steady_clock::now(); + + try { + auto result = verify_async(scitoken); + + // Extract issuer from the result's JWT string after decoding starts + const jwt::decoded_jwt *jwt_decoded = + scitoken.m_decoded.get(); + if (jwt_decoded && jwt_decoded->has_payload_claim("iss")) { + issuer = jwt_decoded->get_issuer(); } - result = verify_async_continue(std::move(result)); + while (!result->m_done) { + auto timeout_val = result->get_timeout_val(expiry_time); + // Limit select to MAX_SELECT_TIMEOUT_US for periodic checks + if (timeout_val.tv_sec > 0 || + timeout_val.tv_usec > MAX_SELECT_TIMEOUT_US) { + timeout_val.tv_sec = 0; + timeout_val.tv_usec = MAX_SELECT_TIMEOUT_US; + } + + select(result->get_max_fd() + 1, result->get_read_fd_set(), + result->get_write_fd_set(), result->get_exc_fd_set(), + &timeout_val); + + if (time(NULL) >= expiry_time) { + throw CurlException( + "Timeout when loading the OIDC metadata."); + } + + result = verify_async_continue(std::move(result)); + } + + // Record successful validation + if (!issuer.empty()) { + auto end_time = std::chrono::steady_clock::now(); + auto duration = + std::chrono::duration_cast>( + end_time - start_time); + internal::MonitoringStats::instance().record_validation_success( + issuer, duration.count()); + } + } catch (const std::exception &e) { + // Record failure if we have an issuer + if (!issuer.empty()) { + auto end_time = std::chrono::steady_clock::now(); + auto duration = + std::chrono::duration_cast>( + end_time - start_time); + record_validation_error(issuer, e, duration.count()); + } + throw; } } void verify(const jwt::decoded_jwt &jwt) { - auto result = verify_async(jwt); - while (!result->m_done) { - result = verify_async_continue(std::move(result)); + std::string issuer = ""; + auto start_time = std::chrono::steady_clock::now(); + + try { + // Try to extract issuer for monitoring + if (jwt.has_payload_claim("iss")) { + issuer = jwt.get_issuer(); + } + + auto result = verify_async(jwt); + while (!result->m_done) { + result = verify_async_continue(std::move(result)); + } + + // Record successful validation + if (!issuer.empty()) { + auto end_time = std::chrono::steady_clock::now(); + auto duration = + std::chrono::duration_cast>( + end_time - start_time); + internal::MonitoringStats::instance().record_validation_success( + issuer, duration.count()); + } + } catch (const std::exception &e) { + // Record failure if we have an issuer + if (!issuer.empty()) { + auto end_time = std::chrono::steady_clock::now(); + auto duration = + std::chrono::duration_cast>( + end_time - start_time); + record_validation_error(issuer, e, duration.count()); + } + throw; } } @@ -514,6 +653,9 @@ class Validator { status->m_jwt_string = jwt.get_token(); status->m_public_pem = public_pem; status->m_algorithm = algorithm; + // Start monitoring timing + status->m_start_time = std::chrono::steady_clock::now(); + status->m_monitoring_started = true; return verify_async_continue(std::move(status)); } @@ -542,7 +684,17 @@ class Validator { const jwt::decoded_jwt jwt( status->m_jwt_string); - verifier.verify(jwt); + try { + verifier.verify(jwt); + } catch (const std::exception &e) { + // Check if this is an expiration error from jwt-cpp + std::string error_msg = e.what(); + if (error_msg.find("exp") != std::string::npos || + error_msg.find("expir") != std::string::npos) { + throw TokenExpiredException(error_msg); + } + throw; + } bool must_verify_everything = true; if (jwt.has_payload_claim("ver")) { @@ -677,6 +829,17 @@ class Validator { } } } + + // Record successful validation + if (status->m_monitoring_started) { + auto end_time = std::chrono::steady_clock::now(); + auto duration = + std::chrono::duration_cast( + end_time - status->m_start_time); + internal::MonitoringStats::instance().record_validation_success( + status->m_issuer, duration.count()); + } + std::unique_ptr result(new AsyncStatus()); result->m_done = true; return result; @@ -802,6 +965,25 @@ class Validator { } } + /** + * Helper method to record monitoring statistics for validation errors. + */ + void record_validation_error(const std::string &issuer, + const std::exception &e, double duration_s) { + // Check exception type instead of string introspection + if (dynamic_cast(&e)) { + internal::MonitoringStats::instance().record_failed_issuer_lookup( + issuer); + } + + if (dynamic_cast(&e)) { + internal::MonitoringStats::instance().record_expired_token(issuer); + } + + internal::MonitoringStats::instance().record_validation_failure( + issuer, duration_s); + } + bool m_validate_all_claims{true}; SciToken::Profile m_profile{SciToken::Profile::COMPAT}; SciToken::Profile m_validate_profile{SciToken::Profile::COMPAT}; diff --git a/src/scitokens_monitoring.cpp b/src/scitokens_monitoring.cpp new file mode 100644 index 0000000..9183033 --- /dev/null +++ b/src/scitokens_monitoring.cpp @@ -0,0 +1,146 @@ +#include "scitokens_internal.h" +#include +#include +#include + +#ifndef PICOJSON_USE_INT64 +#define PICOJSON_USE_INT64 +#endif +#include + +namespace scitokens { +namespace internal { + +MonitoringStats &MonitoringStats::instance() { + static MonitoringStats instance; + return instance; +} + +void MonitoringStats::record_validation_success(const std::string &issuer, + double duration_s) { + std::lock_guard lock(m_mutex); + auto &stats = m_issuer_stats[issuer]; + stats.successful_validations++; + // Add to the total time (accumulate across all validations) + // No atomic needed - protected by mutex + stats.total_time_s += duration_s; +} + +void MonitoringStats::record_validation_failure(const std::string &issuer, + double duration_s) { + std::lock_guard lock(m_mutex); + auto &stats = m_issuer_stats[issuer]; + stats.unsuccessful_validations++; + // Add to the total time (accumulate across all validations) + // No atomic needed - protected by mutex + stats.total_time_s += duration_s; +} + +void MonitoringStats::record_expired_token(const std::string &issuer) { + std::lock_guard lock(m_mutex); + auto &stats = m_issuer_stats[issuer]; + stats.expired_tokens++; +} + +void MonitoringStats::record_failed_issuer_lookup(const std::string &issuer) { + std::lock_guard lock(m_mutex); + + // Limit the number of failed issuer entries to prevent resource exhaustion + if (m_failed_issuer_lookups.size() >= MAX_FAILED_ISSUERS) { + prune_failed_issuers(); + } + + // Only track if we still have room or issuer is already tracked + if (m_failed_issuer_lookups.size() < MAX_FAILED_ISSUERS || + m_failed_issuer_lookups.find(issuer) != m_failed_issuer_lookups.end()) { + m_failed_issuer_lookups[issuer]++; + } +} + +std::string +MonitoringStats::sanitize_issuer_for_json(const std::string &issuer) const { + // Limit issuer length to prevent abuse + const size_t max_length = 256; + std::string sanitized = issuer; + if (sanitized.length() > max_length) { + sanitized = sanitized.substr(0, max_length - 3) + "..."; + } + return sanitized; +} + +void MonitoringStats::prune_failed_issuers() { + // Remove entries with the lowest counts to make room for new ones + if (m_failed_issuer_lookups.empty()) { + return; + } + + // Find the minimum count + uint64_t min_count = UINT64_MAX; + for (const auto &entry : m_failed_issuer_lookups) { + uint64_t count = entry.second; + if (count < min_count) { + min_count = count; + } + } + + // Remove all entries with the minimum count + for (auto it = m_failed_issuer_lookups.begin(); + it != m_failed_issuer_lookups.end();) { + if (it->second == min_count) { + it = m_failed_issuer_lookups.erase(it); + } else { + ++it; + } + } +} + +std::string MonitoringStats::get_json() const { + std::lock_guard lock(m_mutex); + + picojson::object root; + picojson::object issuers_obj; + + // Add per-issuer statistics + for (const auto &entry : m_issuer_stats) { + const std::string &issuer = entry.first; + const IssuerStats &stats = entry.second; + + picojson::object issuer_obj; + issuer_obj["successful_validations"] = picojson::value( + static_cast(stats.successful_validations.load())); + issuer_obj["unsuccessful_validations"] = picojson::value( + static_cast(stats.unsuccessful_validations.load())); + issuer_obj["expired_tokens"] = + picojson::value(static_cast(stats.expired_tokens.load())); + issuer_obj["total_validation_time_s"] = + picojson::value(stats.total_time_s); + + std::string sanitized_issuer = sanitize_issuer_for_json(issuer); + issuers_obj[sanitized_issuer] = picojson::value(issuer_obj); + } + + root["issuers"] = picojson::value(issuers_obj); + + // Add failed issuer lookups + if (!m_failed_issuer_lookups.empty()) { + picojson::object failed_obj; + for (const auto &entry : m_failed_issuer_lookups) { + std::string sanitized_issuer = + sanitize_issuer_for_json(entry.first); + failed_obj[sanitized_issuer] = + picojson::value(static_cast(entry.second)); + } + root["failed_issuer_lookups"] = picojson::value(failed_obj); + } + + return picojson::value(root).serialize(); +} + +void MonitoringStats::reset() { + std::lock_guard lock(m_mutex); + m_issuer_stats.clear(); + m_failed_issuer_lookups.clear(); +} + +} // namespace internal +} // namespace scitokens diff --git a/src/test_monitoring.cpp b/src/test_monitoring.cpp new file mode 100644 index 0000000..b3c6215 --- /dev/null +++ b/src/test_monitoring.cpp @@ -0,0 +1,50 @@ +#include "scitokens.h" +#include +#include + +int main() { + char *json_out = nullptr; + char *err_msg = nullptr; + + // Get initial monitoring statistics (should be empty) + std::cout << "Getting initial monitoring statistics..." << std::endl; + int result = scitoken_get_monitoring_json(&json_out, &err_msg); + if (result != 0) { + std::cerr << "Error getting monitoring JSON: " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) + free(err_msg); + return 1; + } + + std::cout << "Initial statistics: " << json_out << std::endl; + free(json_out); + + // Test reset functionality + std::cout << "\nResetting monitoring statistics..." << std::endl; + result = scitoken_reset_monitoring_stats(&err_msg); + if (result != 0) { + std::cerr << "Error resetting monitoring stats: " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) + free(err_msg); + return 1; + } + + // Get statistics after reset + std::cout << "Getting statistics after reset..." << std::endl; + result = scitoken_get_monitoring_json(&json_out, &err_msg); + if (result != 0) { + std::cerr << "Error getting monitoring JSON: " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) + free(err_msg); + return 1; + } + + std::cout << "Statistics after reset: " << json_out << std::endl; + free(json_out); + + std::cout << "\nMonitoring API test completed successfully!" << std::endl; + return 0; +} diff --git a/src/test_monitoring_comprehensive.cpp b/src/test_monitoring_comprehensive.cpp new file mode 100644 index 0000000..90d007b --- /dev/null +++ b/src/test_monitoring_comprehensive.cpp @@ -0,0 +1,105 @@ +#include "scitokens.h" +#include +#include +#include + +// Helper function to print monitoring JSON +void print_monitoring_stats(const std::string &label) { + char *json_out = nullptr; + char *err_msg = nullptr; + + int result = scitoken_get_monitoring_json(&json_out, &err_msg); + if (result != 0) { + std::cerr << "Error getting monitoring JSON: " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) + free(err_msg); + return; + } + + std::cout << "\n=== " << label << " ===" << std::endl; + std::cout << json_out << std::endl; + free(json_out); +} + +int main() { + char *err_msg = nullptr; + + // Test constants + const int DDOS_TEST_COUNT = 150; // Test beyond MAX_FAILED_ISSUERS limit + + // Reset monitoring stats at start + scitoken_reset_monitoring_stats(&err_msg); + + std::cout << "Testing Monitoring API with Token Validation\n"; + std::cout << "=============================================\n"; + + // Test 1: Initial state + print_monitoring_stats("Initial State (should be empty)"); + + // Test 2: Failed validation - expired token from demo.scitokens.org + // This token is from the test.cpp and is expired + std::cout << "\n--- Test 1: Validating an expired token ---\n"; + std::string expired_token = + "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImtleS1yczI1NiJ9." + "eyJpc3MiOiJodHRwczovL2RlbW8uc2NpdG9rZW5zLm9yZyIsImV4cCI6MTU0NjM5MjAwOS" + "wiaWF0IjoxNTQ2MzkxNDA5LCJuYmYiOjE1NDYzOTE0MDksImp0aSI6ImFkYTk2MjdiLWEx" + "MGYtNGMyYS05Nzc2LTE4ZThkN2JmN2M4NSJ9.cNMG5zI2-JHh7l_" + "PUPUAxom5Vi6Q3akKmv6q57CoVKHtxZAZRc47Uoix_" + "AH3Xzr42qohr2FPamRTxUMsfZjrAFDJ_4JhJ-kKjJ3cRXXF-" + "gj7lbniCDGOBuPXeMsVmeED15nauZ3XKXUHTGLEsg5O6RjS7sGKM_" + "e9YiYvcTvWXcdkrkxZ2dPPU-R3IxdK6PtE9OB2XOk85H670OAJT3qimKm8Dk_" + "Ri6DEEty1Su_" + "1Tov3ac5B19iZkbhhVPMVP0cRolR9UNLhMxQAsbgEmArQOcs046AOzqQz6osOkdYOrVVO7" + "lO2owUyMol94mB_39y1M8jcf5WNq3ukMMIzMCAPwA"; + + SciToken token = nullptr; + int result = scitoken_deserialize(expired_token.c_str(), &token, nullptr, &err_msg); + if (result != 0) { + std::cout << "Token deserialization/validation failed (expected): " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + } else { + std::cout << "Token was valid (unexpected)" << std::endl; + scitoken_destroy(token); + } + + print_monitoring_stats("After Expired Token Validation"); + + // Test 3: Invalid issuer (should not create unbounded entries) + std::cout << "\n--- Test 2: Testing DDoS protection with multiple invalid issuers ---\n"; + + // Try to create many tokens with different invalid issuers + // The monitoring system should limit tracking to MAX_FAILED_ISSUERS (100) + for (int i = 0; i < DDOS_TEST_COUNT; i++) { + // These are malformed tokens that will fail early + std::string fake_token = "invalid.token." + std::to_string(i); + SciToken temp_token = nullptr; + scitoken_deserialize(fake_token.c_str(), &temp_token, nullptr, &err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + } + + print_monitoring_stats("After Multiple Invalid Token Attempts"); + + // Test 4: Reset stats + std::cout << "\n--- Test 3: Testing reset functionality ---\n"; + result = scitoken_reset_monitoring_stats(&err_msg); + if (result != 0) { + std::cerr << "Error resetting stats: " + << (err_msg ? err_msg : "unknown error") << std::endl; + if (err_msg) + free(err_msg); + return 1; + } + + print_monitoring_stats("After Reset"); + + std::cout << "\n=== All monitoring API tests completed successfully ===\n"; + return 0; +} From 181ce60158067b9031bada4b8a3748b739b03b38 Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Tue, 9 Dec 2025 17:47:59 +0000 Subject: [PATCH 2/6] Apply clang-format to fix linter issues - Fix include order in test_monitoring_comprehensive.cpp - Fix trailing whitespace in multiple files - Fix line wrapping for long lines in test files --- src/test_monitoring_comprehensive.cpp | 43 ++++++++++++++------------- test/main.cpp | 20 +++++++------ 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/test_monitoring_comprehensive.cpp b/src/test_monitoring_comprehensive.cpp index 90d007b..9718f64 100644 --- a/src/test_monitoring_comprehensive.cpp +++ b/src/test_monitoring_comprehensive.cpp @@ -1,13 +1,13 @@ #include "scitokens.h" +#include #include #include -#include // Helper function to print monitoring JSON void print_monitoring_stats(const std::string &label) { char *json_out = nullptr; char *err_msg = nullptr; - + int result = scitoken_get_monitoring_json(&json_out, &err_msg); if (result != 0) { std::cerr << "Error getting monitoring JSON: " @@ -16,7 +16,7 @@ void print_monitoring_stats(const std::string &label) { free(err_msg); return; } - + std::cout << "\n=== " << label << " ===" << std::endl; std::cout << json_out << std::endl; free(json_out); @@ -24,19 +24,19 @@ void print_monitoring_stats(const std::string &label) { int main() { char *err_msg = nullptr; - + // Test constants const int DDOS_TEST_COUNT = 150; // Test beyond MAX_FAILED_ISSUERS limit - + // Reset monitoring stats at start scitoken_reset_monitoring_stats(&err_msg); - + std::cout << "Testing Monitoring API with Token Validation\n"; std::cout << "=============================================\n"; - + // Test 1: Initial state print_monitoring_stats("Initial State (should be empty)"); - + // Test 2: Failed validation - expired token from demo.scitokens.org // This token is from the test.cpp and is expired std::cout << "\n--- Test 1: Validating an expired token ---\n"; @@ -52,11 +52,12 @@ int main() { "Ri6DEEty1Su_" "1Tov3ac5B19iZkbhhVPMVP0cRolR9UNLhMxQAsbgEmArQOcs046AOzqQz6osOkdYOrVVO7" "lO2owUyMol94mB_39y1M8jcf5WNq3ukMMIzMCAPwA"; - + SciToken token = nullptr; - int result = scitoken_deserialize(expired_token.c_str(), &token, nullptr, &err_msg); + int result = + scitoken_deserialize(expired_token.c_str(), &token, nullptr, &err_msg); if (result != 0) { - std::cout << "Token deserialization/validation failed (expected): " + std::cout << "Token deserialization/validation failed (expected): " << (err_msg ? err_msg : "unknown error") << std::endl; if (err_msg) { free(err_msg); @@ -66,27 +67,29 @@ int main() { std::cout << "Token was valid (unexpected)" << std::endl; scitoken_destroy(token); } - + print_monitoring_stats("After Expired Token Validation"); - + // Test 3: Invalid issuer (should not create unbounded entries) - std::cout << "\n--- Test 2: Testing DDoS protection with multiple invalid issuers ---\n"; - + std::cout << "\n--- Test 2: Testing DDoS protection with multiple invalid " + "issuers ---\n"; + // Try to create many tokens with different invalid issuers // The monitoring system should limit tracking to MAX_FAILED_ISSUERS (100) for (int i = 0; i < DDOS_TEST_COUNT; i++) { // These are malformed tokens that will fail early std::string fake_token = "invalid.token." + std::to_string(i); SciToken temp_token = nullptr; - scitoken_deserialize(fake_token.c_str(), &temp_token, nullptr, &err_msg); + scitoken_deserialize(fake_token.c_str(), &temp_token, nullptr, + &err_msg); if (err_msg) { free(err_msg); err_msg = nullptr; } } - + print_monitoring_stats("After Multiple Invalid Token Attempts"); - + // Test 4: Reset stats std::cout << "\n--- Test 3: Testing reset functionality ---\n"; result = scitoken_reset_monitoring_stats(&err_msg); @@ -97,9 +100,9 @@ int main() { free(err_msg); return 1; } - + print_monitoring_stats("After Reset"); - + std::cout << "\n=== All monitoring API tests completed successfully ===\n"; return 0; } diff --git a/test/main.cpp b/test/main.cpp index 6107033..0852261 100644 --- a/test/main.cpp +++ b/test/main.cpp @@ -504,7 +504,7 @@ TEST_F(SerializeTest, ExplicitTime) { TEST_F(SerializeTest, GetExpirationErrorHandling) { char *err_msg = nullptr; - + // Test NULL token handling long long expiry; auto rv = scitoken_get_expiration(nullptr, &expiry, &err_msg); @@ -513,27 +513,28 @@ TEST_F(SerializeTest, GetExpirationErrorHandling) { EXPECT_STREQ(err_msg, "Token cannot be NULL"); free(err_msg); err_msg = nullptr; - - // Test NULL expiry parameter handling + + // Test NULL expiry parameter handling rv = scitoken_get_expiration(m_token.get(), nullptr, &err_msg); ASSERT_FALSE(rv == 0); ASSERT_TRUE(err_msg != nullptr); EXPECT_STREQ(err_msg, "Expiry output parameter cannot be NULL"); free(err_msg); err_msg = nullptr; - + // Test normal operation works char *token_value = nullptr; rv = scitoken_serialize(m_token.get(), &token_value, &err_msg); ASSERT_TRUE(rv == 0) << err_msg; - - rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, &err_msg); + + rv = scitoken_deserialize_v2(token_value, m_read_token.get(), nullptr, + &err_msg); ASSERT_TRUE(rv == 0) << err_msg; - + rv = scitoken_get_expiration(m_read_token.get(), &expiry, &err_msg); ASSERT_TRUE(rv == 0) << err_msg; ASSERT_TRUE(expiry > 0); - + free(token_value); } @@ -725,7 +726,8 @@ TEST_F(KeycacheTest, SetGetTest) { TEST_F(KeycacheTest, SetGetConfiguredCacheHome) { // Set cache home char cache_path[FILENAME_MAX]; - ASSERT_TRUE(getcwd(cache_path, sizeof(cache_path)) != nullptr); // Side effect gets cwd + ASSERT_TRUE(getcwd(cache_path, sizeof(cache_path)) != + nullptr); // Side effect gets cwd char *err_msg = nullptr; std::string key = "keycache.cache_home"; From 48672e64618ca14af509c711791620844869cf00 Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Tue, 9 Dec 2025 18:20:11 +0000 Subject: [PATCH 3/6] Improve monitoring tests with counter validation and failure mechanisms - Move monitoring tests from src/ to test/ directory - Add MonitoringStats helper class to parse JSON and validate counters - Add integration tests for monitoring API: - MonitoringCountersIncrease: Verify counters increase after validation - MonitoringFailedIssuerLookup404: Test 404 response handling - MonitoringFailedDNSLookup: Test DNS lookup failure - MonitoringFailedTCPConnection: Test TCP connection failure (port 1) - MonitoringDurationTracking: Verify duration increases over validations - Fix duration unit from nanoseconds to seconds in verify_async_continue - Update CMakeLists.txt to build monitoring tests with gtest --- CMakeLists.txt | 6 - src/scitokens_internal.h | 2 +- src/test_monitoring.cpp | 50 --- src/test_monitoring_comprehensive.cpp | 108 ------ test/CMakeLists.txt | 14 + test/integration_test.cpp | 532 ++++++++++++++++++++++++++ test/monitoring_test.cpp | 244 ++++++++++++ 7 files changed, 791 insertions(+), 165 deletions(-) delete mode 100644 src/test_monitoring.cpp delete mode 100644 src/test_monitoring_comprehensive.cpp create mode 100644 test/monitoring_test.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 117549b..93973fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,12 +80,6 @@ add_executable(scitokens-generate-jwks src/generate_jwks.cpp) target_include_directories(scitokens-generate-jwks PRIVATE ${OPENSSL_INCLUDE_DIRS} ${LIBCRYPTO_INCLUDE_DIRS}) target_link_libraries(scitokens-generate-jwks ${OPENSSL_LIBRARIES} ${LIBCRYPTO_LIBRARIES}) -add_executable(scitokens-test-monitoring src/test_monitoring.cpp) -target_link_libraries(scitokens-test-monitoring SciTokens) - -add_executable(scitokens-test-monitoring-comprehensive src/test_monitoring_comprehensive.cpp) -target_link_libraries(scitokens-test-monitoring-comprehensive SciTokens) - get_directory_property(TARGETS BUILDSYSTEM_TARGETS) install( TARGETS ${TARGETS} diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index ad81f27..4bb4854 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -834,7 +834,7 @@ class Validator { if (status->m_monitoring_started) { auto end_time = std::chrono::steady_clock::now(); auto duration = - std::chrono::duration_cast( + std::chrono::duration_cast>( end_time - status->m_start_time); internal::MonitoringStats::instance().record_validation_success( status->m_issuer, duration.count()); diff --git a/src/test_monitoring.cpp b/src/test_monitoring.cpp deleted file mode 100644 index b3c6215..0000000 --- a/src/test_monitoring.cpp +++ /dev/null @@ -1,50 +0,0 @@ -#include "scitokens.h" -#include -#include - -int main() { - char *json_out = nullptr; - char *err_msg = nullptr; - - // Get initial monitoring statistics (should be empty) - std::cout << "Getting initial monitoring statistics..." << std::endl; - int result = scitoken_get_monitoring_json(&json_out, &err_msg); - if (result != 0) { - std::cerr << "Error getting monitoring JSON: " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) - free(err_msg); - return 1; - } - - std::cout << "Initial statistics: " << json_out << std::endl; - free(json_out); - - // Test reset functionality - std::cout << "\nResetting monitoring statistics..." << std::endl; - result = scitoken_reset_monitoring_stats(&err_msg); - if (result != 0) { - std::cerr << "Error resetting monitoring stats: " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) - free(err_msg); - return 1; - } - - // Get statistics after reset - std::cout << "Getting statistics after reset..." << std::endl; - result = scitoken_get_monitoring_json(&json_out, &err_msg); - if (result != 0) { - std::cerr << "Error getting monitoring JSON: " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) - free(err_msg); - return 1; - } - - std::cout << "Statistics after reset: " << json_out << std::endl; - free(json_out); - - std::cout << "\nMonitoring API test completed successfully!" << std::endl; - return 0; -} diff --git a/src/test_monitoring_comprehensive.cpp b/src/test_monitoring_comprehensive.cpp deleted file mode 100644 index 9718f64..0000000 --- a/src/test_monitoring_comprehensive.cpp +++ /dev/null @@ -1,108 +0,0 @@ -#include "scitokens.h" -#include -#include -#include - -// Helper function to print monitoring JSON -void print_monitoring_stats(const std::string &label) { - char *json_out = nullptr; - char *err_msg = nullptr; - - int result = scitoken_get_monitoring_json(&json_out, &err_msg); - if (result != 0) { - std::cerr << "Error getting monitoring JSON: " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) - free(err_msg); - return; - } - - std::cout << "\n=== " << label << " ===" << std::endl; - std::cout << json_out << std::endl; - free(json_out); -} - -int main() { - char *err_msg = nullptr; - - // Test constants - const int DDOS_TEST_COUNT = 150; // Test beyond MAX_FAILED_ISSUERS limit - - // Reset monitoring stats at start - scitoken_reset_monitoring_stats(&err_msg); - - std::cout << "Testing Monitoring API with Token Validation\n"; - std::cout << "=============================================\n"; - - // Test 1: Initial state - print_monitoring_stats("Initial State (should be empty)"); - - // Test 2: Failed validation - expired token from demo.scitokens.org - // This token is from the test.cpp and is expired - std::cout << "\n--- Test 1: Validating an expired token ---\n"; - std::string expired_token = - "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsImtpZCI6ImtleS1yczI1NiJ9." - "eyJpc3MiOiJodHRwczovL2RlbW8uc2NpdG9rZW5zLm9yZyIsImV4cCI6MTU0NjM5MjAwOS" - "wiaWF0IjoxNTQ2MzkxNDA5LCJuYmYiOjE1NDYzOTE0MDksImp0aSI6ImFkYTk2MjdiLWEx" - "MGYtNGMyYS05Nzc2LTE4ZThkN2JmN2M4NSJ9.cNMG5zI2-JHh7l_" - "PUPUAxom5Vi6Q3akKmv6q57CoVKHtxZAZRc47Uoix_" - "AH3Xzr42qohr2FPamRTxUMsfZjrAFDJ_4JhJ-kKjJ3cRXXF-" - "gj7lbniCDGOBuPXeMsVmeED15nauZ3XKXUHTGLEsg5O6RjS7sGKM_" - "e9YiYvcTvWXcdkrkxZ2dPPU-R3IxdK6PtE9OB2XOk85H670OAJT3qimKm8Dk_" - "Ri6DEEty1Su_" - "1Tov3ac5B19iZkbhhVPMVP0cRolR9UNLhMxQAsbgEmArQOcs046AOzqQz6osOkdYOrVVO7" - "lO2owUyMol94mB_39y1M8jcf5WNq3ukMMIzMCAPwA"; - - SciToken token = nullptr; - int result = - scitoken_deserialize(expired_token.c_str(), &token, nullptr, &err_msg); - if (result != 0) { - std::cout << "Token deserialization/validation failed (expected): " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) { - free(err_msg); - err_msg = nullptr; - } - } else { - std::cout << "Token was valid (unexpected)" << std::endl; - scitoken_destroy(token); - } - - print_monitoring_stats("After Expired Token Validation"); - - // Test 3: Invalid issuer (should not create unbounded entries) - std::cout << "\n--- Test 2: Testing DDoS protection with multiple invalid " - "issuers ---\n"; - - // Try to create many tokens with different invalid issuers - // The monitoring system should limit tracking to MAX_FAILED_ISSUERS (100) - for (int i = 0; i < DDOS_TEST_COUNT; i++) { - // These are malformed tokens that will fail early - std::string fake_token = "invalid.token." + std::to_string(i); - SciToken temp_token = nullptr; - scitoken_deserialize(fake_token.c_str(), &temp_token, nullptr, - &err_msg); - if (err_msg) { - free(err_msg); - err_msg = nullptr; - } - } - - print_monitoring_stats("After Multiple Invalid Token Attempts"); - - // Test 4: Reset stats - std::cout << "\n--- Test 3: Testing reset functionality ---\n"; - result = scitoken_reset_monitoring_stats(&err_msg); - if (result != 0) { - std::cerr << "Error resetting stats: " - << (err_msg ? err_msg : "unknown error") << std::endl; - if (err_msg) - free(err_msg); - return 1; - } - - print_monitoring_stats("After Reset"); - - std::cout << "\n=== All monitoring API tests completed successfully ===\n"; - return 0; -} diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 56f3494..bdd31e7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -20,6 +20,20 @@ add_test( ${CMAKE_CURRENT_BINARY_DIR}/scitokens-gtest ) +# Monitoring unit tests +add_executable(scitokens-monitoring-test monitoring_test.cpp) +if( NOT SCITOKENS_EXTERNAL_GTEST ) + add_dependencies(scitokens-monitoring-test gtest) +endif() +target_link_libraries(scitokens-monitoring-test SciTokens "${LIBGTEST}" pthread) + +add_test( + NAME + monitoring + COMMAND + ${CMAKE_CURRENT_BINARY_DIR}/scitokens-monitoring-test + ) + # Integration test executable add_executable(scitokens-integration-test integration_test.cpp) if( NOT SCITOKENS_EXTERNAL_GTEST ) diff --git a/test/integration_test.cpp b/test/integration_test.cpp index 4b308ae..5cf0047 100644 --- a/test/integration_test.cpp +++ b/test/integration_test.cpp @@ -1,5 +1,6 @@ #include "../src/scitokens.h" +#include #include #include #include @@ -9,8 +10,138 @@ #include #include +#ifndef PICOJSON_USE_INT64 +#define PICOJSON_USE_INT64 +#endif +#include + namespace { +// Helper class to parse monitoring JSON +class MonitoringStats { + public: + struct IssuerStats { + uint64_t successful_validations{0}; + uint64_t unsuccessful_validations{0}; + uint64_t expired_tokens{0}; + double total_validation_time_s{0.0}; + }; + + bool parse(const std::string &json) { + picojson::value root; + std::string err = picojson::parse(root, json); + if (!err.empty()) { + return false; + } + + if (!root.is()) { + return false; + } + + auto &root_obj = root.get(); + + // Parse issuers + issuers_.clear(); + auto issuers_it = root_obj.find("issuers"); + if (issuers_it != root_obj.end() && + issuers_it->second.is()) { + auto &issuers_obj = issuers_it->second.get(); + for (const auto &issuer_entry : issuers_obj) { + if (issuer_entry.second.is()) { + IssuerStats stats; + auto &stats_obj = + issuer_entry.second.get(); + + auto it = stats_obj.find("successful_validations"); + if (it != stats_obj.end() && it->second.is()) { + stats.successful_validations = + static_cast(it->second.get()); + } + + it = stats_obj.find("unsuccessful_validations"); + if (it != stats_obj.end() && it->second.is()) { + stats.unsuccessful_validations = + static_cast(it->second.get()); + } + + it = stats_obj.find("expired_tokens"); + if (it != stats_obj.end() && it->second.is()) { + stats.expired_tokens = + static_cast(it->second.get()); + } + + it = stats_obj.find("total_validation_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.total_validation_time_s = + it->second.get(); + } + + issuers_[issuer_entry.first] = stats; + } + } + } + + // Parse failed issuer lookups + failed_issuer_lookups_.clear(); + auto failed_it = root_obj.find("failed_issuer_lookups"); + if (failed_it != root_obj.end() && + failed_it->second.is()) { + auto &failed_obj = failed_it->second.get(); + for (const auto &entry : failed_obj) { + if (entry.second.is()) { + failed_issuer_lookups_[entry.first] = + static_cast(entry.second.get()); + } + } + } + + return true; + } + + IssuerStats getIssuerStats(const std::string &issuer) const { + auto it = issuers_.find(issuer); + if (it != issuers_.end()) { + return it->second; + } + return IssuerStats{}; + } + + uint64_t getFailedLookupCount(const std::string &issuer) const { + auto it = failed_issuer_lookups_.find(issuer); + if (it != failed_issuer_lookups_.end()) { + return it->second; + } + return 0; + } + + size_t getIssuerCount() const { return issuers_.size(); } + + size_t getFailedIssuerCount() const { + return failed_issuer_lookups_.size(); + } + + private: + std::map issuers_; + std::map failed_issuer_lookups_; +}; + +// Helper to get current monitoring stats +MonitoringStats getCurrentMonitoringStats() { + char *json_out = nullptr; + char *err_msg = nullptr; + MonitoringStats stats; + + int rv = scitoken_get_monitoring_json(&json_out, &err_msg); + if (rv == 0 && json_out) { + stats.parse(json_out); + free(json_out); + } + if (err_msg) + free(err_msg); + + return stats; +} + // Helper to read environment variables from setup.sh class TestEnvironment { public: @@ -380,6 +511,407 @@ TEST_F(IntegrationTest, EnforcerWithDynamicIssuer) { enforcer_destroy(enforcer); } +// ============================================================================= +// Monitoring API Integration Tests +// ============================================================================= + +TEST_F(IntegrationTest, MonitoringCountersIncrease) { + char *err_msg = nullptr; + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Get initial stats + auto initial_stats = getCurrentMonitoringStats(); + auto initial_issuer_stats = initial_stats.getIssuerStats(issuer_url_); + + // Create and verify a valid token + std::unique_ptr key( + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg), + scitoken_key_destroy); + ASSERT_TRUE(key.get() != nullptr); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + std::unique_ptr token( + scitoken_create(key.get()), scitoken_destroy); + ASSERT_TRUE(token.get() != nullptr); + + auto rv = scitoken_set_claim_string(token.get(), "iss", issuer_url_.c_str(), + &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + rv = + scitoken_set_claim_string(token.get(), "sub", "test-subject", &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + scitoken_set_lifetime(token.get(), 3600); + + char *token_value = nullptr; + rv = scitoken_serialize(token.get(), &token_value, &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + std::unique_ptr token_value_ptr(token_value, free); + + // Verify the token (should increment successful_validations) + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Check that counters increased + auto after_stats = getCurrentMonitoringStats(); + auto after_issuer_stats = after_stats.getIssuerStats(issuer_url_); + + EXPECT_GT(after_issuer_stats.successful_validations, + initial_issuer_stats.successful_validations) + << "successful_validations should have increased"; + + // Duration should also have increased + EXPECT_GT(after_issuer_stats.total_validation_time_s, + initial_issuer_stats.total_validation_time_s) + << "total_validation_time_s should have increased"; + + std::cout << "After successful validation:" << std::endl; + std::cout << " successful_validations: " + << after_issuer_stats.successful_validations << std::endl; + std::cout << " total_validation_time_s: " + << after_issuer_stats.total_validation_time_s << std::endl; +} + +TEST_F(IntegrationTest, MonitoringFailedIssuerLookup404) { + char *err_msg = nullptr; + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Parse the issuer URL to construct a 404 path + // The server returns 404 for paths other than + // /.well-known/openid-configuration We need to use the same host but a path + // that doesn't exist + std::string issuer_404 = issuer_url_ + "/nonexistent-path"; + + // Create a token with issuer that will get 404 + std::unique_ptr key( + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg), + scitoken_key_destroy); + ASSERT_TRUE(key.get() != nullptr); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + std::unique_ptr token( + scitoken_create(key.get()), scitoken_destroy); + ASSERT_TRUE(token.get() != nullptr); + + // Use issuer URL that will cause 404 on metadata lookup + auto rv = scitoken_set_claim_string(token.get(), "iss", issuer_404.c_str(), + &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + scitoken_set_lifetime(token.get(), 3600); + + char *token_value = nullptr; + rv = scitoken_serialize(token.get(), &token_value, &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + std::unique_ptr token_value_ptr(token_value, free); + + // Try to verify - should fail with 404 + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + EXPECT_NE(rv, 0) << "Verification should fail for 404 issuer"; + if (err_msg) { + std::cout << "Expected error: " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + + // Check that failed issuer lookup was recorded + auto stats = getCurrentMonitoringStats(); + auto issuer_stats = stats.getIssuerStats(issuer_404); + + EXPECT_GT(issuer_stats.unsuccessful_validations, 0u) + << "unsuccessful_validations should have increased for 404 issuer"; + + std::cout << "After 404 response:" << std::endl; + std::cout << " unsuccessful_validations: " + << issuer_stats.unsuccessful_validations << std::endl; +} + +TEST_F(IntegrationTest, MonitoringFailedDNSLookup) { + char *err_msg = nullptr; + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Use an issuer with a hostname that won't resolve + std::string dns_fail_issuer = + "https://this-hostname-does-not-exist-12345.invalid"; + + // Create a token with issuer that will fail DNS lookup + std::unique_ptr key( + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg), + scitoken_key_destroy); + ASSERT_TRUE(key.get() != nullptr); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + std::unique_ptr token( + scitoken_create(key.get()), scitoken_destroy); + ASSERT_TRUE(token.get() != nullptr); + + auto rv = scitoken_set_claim_string(token.get(), "iss", + dns_fail_issuer.c_str(), &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + scitoken_set_lifetime(token.get(), 3600); + + char *token_value = nullptr; + rv = scitoken_serialize(token.get(), &token_value, &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + std::unique_ptr token_value_ptr(token_value, free); + + // Try to verify - should fail with DNS error + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + EXPECT_NE(rv, 0) << "Verification should fail for DNS lookup failure"; + if (err_msg) { + std::cout << "Expected error (DNS): " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + + // Check that failed issuer lookup was recorded + auto stats = getCurrentMonitoringStats(); + auto issuer_stats = stats.getIssuerStats(dns_fail_issuer); + + EXPECT_GT(issuer_stats.unsuccessful_validations, 0u) + << "unsuccessful_validations should have increased for DNS failure"; + + std::cout << "After DNS failure:" << std::endl; + std::cout << " unsuccessful_validations: " + << issuer_stats.unsuccessful_validations << std::endl; +} + +TEST_F(IntegrationTest, MonitoringFailedTCPConnection) { + char *err_msg = nullptr; + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Use localhost with a privileged port (< 1024) that won't have a server + // Port 1 is typically not used and requires root to bind + std::string tcp_fail_issuer = "https://localhost:1"; + + // Create a token with issuer that will fail TCP connection + std::unique_ptr key( + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg), + scitoken_key_destroy); + ASSERT_TRUE(key.get() != nullptr); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + std::unique_ptr token( + scitoken_create(key.get()), scitoken_destroy); + ASSERT_TRUE(token.get() != nullptr); + + auto rv = scitoken_set_claim_string(token.get(), "iss", + tcp_fail_issuer.c_str(), &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + scitoken_set_lifetime(token.get(), 3600); + + char *token_value = nullptr; + rv = scitoken_serialize(token.get(), &token_value, &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + std::unique_ptr token_value_ptr(token_value, free); + + // Try to verify - should fail with connection refused + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + EXPECT_NE(rv, 0) << "Verification should fail for TCP connection failure"; + if (err_msg) { + std::cout << "Expected error (TCP): " << err_msg << std::endl; + free(err_msg); + err_msg = nullptr; + } + + // Check that failed issuer lookup was recorded + auto stats = getCurrentMonitoringStats(); + auto issuer_stats = stats.getIssuerStats(tcp_fail_issuer); + + EXPECT_GT(issuer_stats.unsuccessful_validations, 0u) + << "unsuccessful_validations should have increased for TCP failure"; + + std::cout << "After TCP connection failure:" << std::endl; + std::cout << " unsuccessful_validations: " + << issuer_stats.unsuccessful_validations << std::endl; +} + +TEST_F(IntegrationTest, MonitoringDurationTracking) { + char *err_msg = nullptr; + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Perform multiple validations and check duration increases + for (int i = 0; i < 3; i++) { + std::unique_ptr key( + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg), + scitoken_key_destroy); + ASSERT_TRUE(key.get() != nullptr); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + std::unique_ptr token( + scitoken_create(key.get()), scitoken_destroy); + ASSERT_TRUE(token.get() != nullptr); + + auto rv = scitoken_set_claim_string(token.get(), "iss", + issuer_url_.c_str(), &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + rv = scitoken_set_claim_string(token.get(), "sub", "test-subject", + &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + scitoken_set_lifetime(token.get(), 3600); + + char *token_value = nullptr; + rv = scitoken_serialize(token.get(), &token_value, &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + std::unique_ptr token_value_ptr(token_value, + free); + + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + ASSERT_EQ(rv, 0); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + } + + // Check final stats + auto stats = getCurrentMonitoringStats(); + auto issuer_stats = stats.getIssuerStats(issuer_url_); + + EXPECT_GE(issuer_stats.successful_validations, 3u) + << "Should have at least 3 successful validations"; + EXPECT_GT(issuer_stats.total_validation_time_s, 0.0) + << "total_validation_time_s should be positive"; + + std::cout << "After multiple validations:" << std::endl; + std::cout << " successful_validations: " + << issuer_stats.successful_validations << std::endl; + std::cout << " total_validation_time_s: " + << issuer_stats.total_validation_time_s << std::endl; +} + } // namespace int main(int argc, char **argv) { diff --git a/test/monitoring_test.cpp b/test/monitoring_test.cpp new file mode 100644 index 0000000..7d07fb7 --- /dev/null +++ b/test/monitoring_test.cpp @@ -0,0 +1,244 @@ +/** + * Monitoring API unit tests + * + * Tests the monitoring API for per-issuer validation statistics including: + * - Counter increments for successful/unsuccessful validations + * - Duration tracking + * - Failed issuer lookup tracking + * - DDoS protection (max entries limit) + * - Reset functionality + */ + +#include "../src/scitokens.h" + +#include +#include +#include + +#ifndef PICOJSON_USE_INT64 +#define PICOJSON_USE_INT64 +#endif +#include + +namespace { + +// Helper class to parse monitoring JSON +class MonitoringStats { + public: + struct IssuerStats { + uint64_t successful_validations{0}; + uint64_t unsuccessful_validations{0}; + uint64_t expired_tokens{0}; + double total_validation_time_s{0.0}; + }; + + bool parse(const std::string &json) { + picojson::value root; + std::string err = picojson::parse(root, json); + if (!err.empty()) { + return false; + } + + if (!root.is()) { + return false; + } + + auto &root_obj = root.get(); + + // Parse issuers + issuers_.clear(); + auto issuers_it = root_obj.find("issuers"); + if (issuers_it != root_obj.end() && + issuers_it->second.is()) { + auto &issuers_obj = issuers_it->second.get(); + for (const auto &issuer_entry : issuers_obj) { + if (issuer_entry.second.is()) { + IssuerStats stats; + auto &stats_obj = + issuer_entry.second.get(); + + auto it = stats_obj.find("successful_validations"); + if (it != stats_obj.end() && it->second.is()) { + stats.successful_validations = + static_cast(it->second.get()); + } + + it = stats_obj.find("unsuccessful_validations"); + if (it != stats_obj.end() && it->second.is()) { + stats.unsuccessful_validations = + static_cast(it->second.get()); + } + + it = stats_obj.find("expired_tokens"); + if (it != stats_obj.end() && it->second.is()) { + stats.expired_tokens = + static_cast(it->second.get()); + } + + it = stats_obj.find("total_validation_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.total_validation_time_s = + it->second.get(); + } + + issuers_[issuer_entry.first] = stats; + } + } + } + + // Parse failed issuer lookups + failed_issuer_lookups_.clear(); + auto failed_it = root_obj.find("failed_issuer_lookups"); + if (failed_it != root_obj.end() && + failed_it->second.is()) { + auto &failed_obj = failed_it->second.get(); + for (const auto &entry : failed_obj) { + if (entry.second.is()) { + failed_issuer_lookups_[entry.first] = + static_cast(entry.second.get()); + } + } + } + + return true; + } + + IssuerStats getIssuerStats(const std::string &issuer) const { + auto it = issuers_.find(issuer); + if (it != issuers_.end()) { + return it->second; + } + return IssuerStats{}; + } + + uint64_t getFailedLookupCount(const std::string &issuer) const { + auto it = failed_issuer_lookups_.find(issuer); + if (it != failed_issuer_lookups_.end()) { + return it->second; + } + return 0; + } + + size_t getIssuerCount() const { return issuers_.size(); } + + size_t getFailedIssuerCount() const { + return failed_issuer_lookups_.size(); + } + + private: + std::map issuers_; + std::map failed_issuer_lookups_; +}; + +// Helper to get current monitoring stats +MonitoringStats getCurrentStats() { + char *json_out = nullptr; + char *err_msg = nullptr; + MonitoringStats stats; + + int rv = scitoken_get_monitoring_json(&json_out, &err_msg); + if (rv == 0 && json_out) { + stats.parse(json_out); + free(json_out); + } + if (err_msg) + free(err_msg); + + return stats; +} + +class MonitoringTest : public ::testing::Test { + protected: + void SetUp() override { + // Reset monitoring stats before each test + char *err_msg = nullptr; + scitoken_reset_monitoring_stats(&err_msg); + if (err_msg) + free(err_msg); + } +}; + +TEST_F(MonitoringTest, GetMonitoringJson) { + char *json_out = nullptr; + char *err_msg = nullptr; + + int rv = scitoken_get_monitoring_json(&json_out, &err_msg); + ASSERT_EQ(rv, 0) << "Failed to get monitoring JSON: " + << (err_msg ? err_msg : "unknown"); + ASSERT_NE(json_out, nullptr); + + // Should be valid JSON with "issuers" key + MonitoringStats stats; + EXPECT_TRUE(stats.parse(json_out)); + EXPECT_EQ(stats.getIssuerCount(), 0); // Should be empty after reset + + free(json_out); + if (err_msg) + free(err_msg); +} + +TEST_F(MonitoringTest, GetMonitoringJsonNullOutput) { + char *err_msg = nullptr; + + int rv = scitoken_get_monitoring_json(nullptr, &err_msg); + EXPECT_NE(rv, 0); + EXPECT_NE(err_msg, nullptr); + + if (err_msg) + free(err_msg); +} + +TEST_F(MonitoringTest, ResetMonitoringStats) { + char *err_msg = nullptr; + + int rv = scitoken_reset_monitoring_stats(&err_msg); + EXPECT_EQ(rv, 0); + + auto stats = getCurrentStats(); + EXPECT_EQ(stats.getIssuerCount(), 0); + EXPECT_EQ(stats.getFailedIssuerCount(), 0); + + if (err_msg) + free(err_msg); +} + +TEST_F(MonitoringTest, DDoSProtection) { + // The monitoring system should limit tracking failed issuers to + // MAX_FAILED_ISSUERS (100) + const int DDOS_TEST_COUNT = 150; + char *err_msg = nullptr; + + // Try to create many tokens with different invalid issuers + for (int i = 0; i < DDOS_TEST_COUNT; i++) { + std::string fake_token = "invalid.token." + std::to_string(i); + SciToken temp_token = nullptr; + scitoken_deserialize(fake_token.c_str(), &temp_token, nullptr, + &err_msg); + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + } + + auto stats = getCurrentStats(); + + // The system should have limited entries to prevent resource exhaustion + // We can't check exact count since malformed tokens may fail before issuer + // extraction, but we should verify the system didn't crash and stats work + char *json_out = nullptr; + int rv = scitoken_get_monitoring_json(&json_out, &err_msg); + EXPECT_EQ(rv, 0); + EXPECT_NE(json_out, nullptr); + + if (json_out) + free(json_out); + if (err_msg) + free(err_msg); +} + +} // namespace + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} From ac2d80d3c90c764f96b90d3f3947c0defe3bedc5 Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Wed, 10 Dec 2025 12:43:55 +0000 Subject: [PATCH 4/6] Refactor monitoring: periodic duration updates in blocking verify() - Add sync/async validation started counters (sync_validations_started, async_validations_started) separate from result counters - Add sync/async duration tracking (sync_total_time_ns, async_total_time_ns) - Update blocking verify() to track sync starts and update duration every 50ms via select timeout, only calling verify_async_continue on actual I/O - Update verify_async() to track async starts - Add m_is_sync flag to AsyncStatus to prevent double-counting - Add MAX_SELECT_TIMEOUT_US constant (50ms) for periodic checks - Update get_json() to output new sync/async fields - Update tests to parse new JSON fields --- src/scitokens.h | 22 +++- src/scitokens_internal.cpp | 25 +++- src/scitokens_internal.h | 243 ++++++++++++++++++++++++++++------- src/scitokens_monitoring.cpp | 77 ++++++----- test/integration_test.cpp | 116 ++++++++++++++++- test/monitoring_test.cpp | 116 ++++++++++++++++- 6 files changed, 498 insertions(+), 101 deletions(-) diff --git a/src/scitokens.h b/src/scitokens.h index 4d05202..7970043 100644 --- a/src/scitokens.h +++ b/src/scitokens.h @@ -331,13 +331,27 @@ int scitoken_config_get_str(const char *key, char **output, char **err_msg); /** * Get monitoring statistics as a JSON string. - * Returns a JSON object containing per-issuer validation statistics including: + * Returns a JSON object containing per-issuer validation statistics. + * + * Per-issuer statistics (under "issuers" key): * - successful_validations: count of successful token validations * - unsuccessful_validations: count of failed token validations * - expired_tokens: count of expired tokens encountered - * - total_validation_time_s: total validation time in seconds - * - failed_issuer_lookups: count of failed issuer lookups (limited to prevent - * DDoS) + * - sync_validations_started: count of validations started via blocking API + * - async_validations_started: count of validations started via async API + * - sync_total_time_s: time spent in blocking verify() calls (updated every 50ms) + * - async_total_time_s: time spent in async validations (updated on completion) + * - total_validation_time_s: sum of sync and async time + * - successful_key_lookups: count of successful JWKS web refreshes + * - failed_key_lookups: count of failed JWKS web refreshes + * - failed_key_lookup_time_s: total time spent on failed key lookups + * - expired_keys: count of times keys expired before refresh completed + * - failed_refreshes: count of failed key refresh attempts (used cached keys) + * - stale_key_uses: count of times keys were used past their next_update time + * + * Failed issuer lookups (under "failed_issuer_lookups" key): + * - Per unknown issuer: count and total_time_s of failed lookup attempts + * - Limited to 100 entries to prevent resource exhaustion from DDoS attacks * * The returned string must be freed by the caller using free(). * Returns 0 on success, nonzero on failure. diff --git a/src/scitokens_internal.cpp b/src/scitokens_internal.cpp index 67116ce..15a74a4 100644 --- a/src/scitokens_internal.cpp +++ b/src/scitokens_internal.cpp @@ -828,14 +828,23 @@ Validator::get_public_key_pem(const std::string &issuer, const std::string &kid, std::unique_lock lock(key_refresh_mutex, std::defer_lock); // If refresh is due *and* the key refresh mutex is free, try to update if (now > result->m_next_update && lock.try_lock()) { + // Get a reference to this issuer's statistics + auto &issuer_stats = + internal::MonitoringStats::instance().get_issuer_stats(issuer); + // Record that we're using a stale key (past next_update) + issuer_stats.inc_stale_key_use(); try { result->m_ignore_error = true; result = get_public_keys_from_web( issuer, internal::SimpleCurlGet::default_timeout); // Hold refresh mutex in the new result result->m_refresh_lock = std::move(lock); + // Mark that this is a refresh attempt for a known issuer + result->m_is_refresh = true; } catch (std::runtime_error &) { result->m_do_store = false; + // Record failed refresh for known issuer + issuer_stats.inc_failed_refresh(); // ignore the exception: we have a valid set of keys already } } else { @@ -845,7 +854,14 @@ Validator::get_public_key_pem(const std::string &issuer, const std::string &kid, result->m_done = true; } } else { - // No keys in the DB, or they are expired, so get them from the web. + // No keys in the DB, or they are expired + // Record that we had expired keys if the issuer was previously known + // (This is tracked by having an entry in issuer stats) + auto &issuer_stats = + internal::MonitoringStats::instance().get_issuer_stats(issuer); + issuer_stats.inc_expired_key(); + + // Get keys from the web. result = get_public_keys_from_web( issuer, internal::SimpleCurlGet::default_timeout); } @@ -869,6 +885,13 @@ Validator::get_public_key_pem_continue(std::unique_ptr status, } } if (status->m_do_store) { + // Async web fetch completed successfully - record monitoring + if (status->m_is_refresh) { + auto &issuer_stats = + internal::MonitoringStats::instance().get_issuer_stats( + status->m_issuer); + issuer_stats.inc_successful_key_lookup(); + } store_public_keys(status->m_issuer, status->m_keys, status->m_next_update, status->m_expires); } diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index 4bb4854..6244671 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -116,12 +116,99 @@ class SimpleCurlGet { /** * Statistics for monitoring token validation per issuer. + * All counters are atomic for thread-safe access. + * Time values are stored in nanoseconds internally for atomic operations. */ struct IssuerStats { + // Validation result counters std::atomic successful_validations{0}; std::atomic unsuccessful_validations{0}; std::atomic expired_tokens{0}; - double total_time_s{0.0}; // Total time in seconds - protected by mutex + + // Validation started counters (separate from results) + std::atomic sync_validations_started{0}; // Started via blocking verify() + std::atomic async_validations_started{0}; // Started via verify_async() + + // Duration tracking (nanoseconds) + // sync_total_time_ns is updated periodically during blocking verify() + std::atomic sync_total_time_ns{0}; + // async_total_time_ns is only updated on completion + std::atomic async_total_time_ns{0}; + + // Key lookup statistics + std::atomic successful_key_lookups{0}; + std::atomic failed_key_lookups{0}; + std::atomic failed_key_lookup_time_ns{0}; // In nanoseconds + + // Key refresh statistics + std::atomic expired_keys{0}; + std::atomic failed_refreshes{0}; + std::atomic stale_key_uses{0}; + + // Increment methods for atomic counters + void inc_successful_validation() { successful_validations++; } + void inc_unsuccessful_validation() { unsuccessful_validations++; } + void inc_expired_token() { expired_tokens++; } + void inc_sync_validation_started() { sync_validations_started++; } + void inc_async_validation_started() { async_validations_started++; } + void inc_stale_key_use() { stale_key_uses++; } + void inc_failed_refresh() { failed_refreshes++; } + void inc_expired_key() { expired_keys++; } + void inc_successful_key_lookup() { successful_key_lookups++; } + void inc_failed_key_lookup() { failed_key_lookups++; } + + // Time setters that accept std::chrono::duration + template + void add_sync_time(std::chrono::duration duration) { + auto ns = + std::chrono::duration_cast(duration); + sync_total_time_ns += static_cast(ns.count()); + } + + template + void add_async_time(std::chrono::duration duration) { + auto ns = + std::chrono::duration_cast(duration); + async_total_time_ns += static_cast(ns.count()); + } + + template + void + add_failed_key_lookup_time(std::chrono::duration duration) { + auto ns = + std::chrono::duration_cast(duration); + failed_key_lookup_time_ns += static_cast(ns.count()); + } + + void inc_failed_key_lookup(std::chrono::nanoseconds duration) { + failed_key_lookups++; + failed_key_lookup_time_ns += static_cast(duration.count()); + } + + // Time getters that return seconds as double + double get_sync_time_s() const { + return static_cast(sync_total_time_ns.load()) / 1e9; + } + + double get_async_time_s() const { + return static_cast(async_total_time_ns.load()) / 1e9; + } + + double get_total_time_s() const { + return get_sync_time_s() + get_async_time_s(); + } + + double get_failed_key_lookup_time_s() const { + return static_cast(failed_key_lookup_time_ns.load()) / 1e9; + } +}; + +/** + * Statistics for failed (unknown) issuer lookups. + */ +struct FailedIssuerStats { + uint64_t count{0}; + double total_time_s{0.0}; }; /** @@ -133,12 +220,22 @@ class MonitoringStats { public: static MonitoringStats &instance(); - void record_validation_success(const std::string &issuer, - double duration_s); - void record_validation_failure(const std::string &issuer, - double duration_s); - void record_expired_token(const std::string &issuer); - void record_failed_issuer_lookup(const std::string &issuer); + /** + * Get a reference to an issuer's statistics, creating the entry if needed. + * The returned reference remains valid for the lifetime of the singleton. + * All IssuerStats fields are atomic, so concurrent access is safe. + */ + IssuerStats &get_issuer_stats(const std::string &issuer) { + std::lock_guard lock(m_mutex); + return m_issuer_stats[issuer]; + } + + /** + * Record a failed issuer lookup (for unknown/invalid issuers). + * This uses a separate map with DDoS protection. + */ + void record_failed_issuer_lookup(const std::string &issuer, + double duration_s); std::string get_json() const; void reset(); @@ -154,7 +251,7 @@ class MonitoringStats { mutable std::mutex m_mutex; std::unordered_map m_issuer_stats; - std::unordered_map m_failed_issuer_lookups; + std::unordered_map m_failed_issuer_lookups; std::string sanitize_issuer_for_json(const std::string &issuer) const; void prune_failed_issuers(); @@ -275,6 +372,7 @@ class AsyncStatus { bool m_do_store{true}; bool m_has_metadata{false}; bool m_oauth_fallback{false}; + bool m_is_refresh{false}; // True if this is a refresh of an existing key AsyncState m_state{DOWNLOAD_METADATA}; std::unique_lock m_refresh_lock; @@ -290,6 +388,7 @@ class AsyncStatus { std::string m_algorithm; std::chrono::steady_clock::time_point m_start_time; bool m_monitoring_started{false}; + bool m_is_sync{false}; // True if called from blocking verify(), false for pure async struct timeval get_timeout_val(time_t expiry_time) const { auto now = time(NULL); @@ -487,6 +586,8 @@ class Validator { void verify(const SciToken &scitoken, time_t expiry_time) { std::string issuer = ""; auto start_time = std::chrono::steady_clock::now(); + auto last_duration_update = start_time; + internal::IssuerStats *issuer_stats = nullptr; try { auto result = verify_async(scitoken); @@ -496,6 +597,11 @@ class Validator { scitoken.m_decoded.get(); if (jwt_decoded && jwt_decoded->has_payload_claim("iss")) { issuer = jwt_decoded->get_issuer(); + // Record sync validation started and get stats reference + issuer_stats = + &internal::MonitoringStats::instance().get_issuer_stats( + issuer); + issuer_stats->inc_sync_validation_started(); } while (!result->m_done) { @@ -507,35 +613,59 @@ class Validator { timeout_val.tv_usec = MAX_SELECT_TIMEOUT_US; } - select(result->get_max_fd() + 1, result->get_read_fd_set(), - result->get_write_fd_set(), result->get_exc_fd_set(), - &timeout_val); + int select_result = + select(result->get_max_fd() + 1, result->get_read_fd_set(), + result->get_write_fd_set(), result->get_exc_fd_set(), + &timeout_val); + + // Update duration periodically on each select return + if (issuer_stats) { + auto now = std::chrono::steady_clock::now(); + auto delta = std::chrono::duration_cast< + std::chrono::nanoseconds>(now - last_duration_update); + issuer_stats->add_sync_time(delta); + last_duration_update = now; + } if (time(NULL) >= expiry_time) { throw CurlException( "Timeout when loading the OIDC metadata."); } - result = verify_async_continue(std::move(result)); + // Only continue if select returned due to I/O activity (not timeout) + if (select_result > 0) { + result = verify_async_continue(std::move(result)); + } + // If select_result == 0 (timeout) or -1 (error/interrupt), + // just loop back to update duration and check expiry } - // Record successful validation - if (!issuer.empty()) { + // Record successful validation (final duration update) + if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); - auto duration = - std::chrono::duration_cast>( - end_time - start_time); - internal::MonitoringStats::instance().record_validation_success( - issuer, duration.count()); + auto delta = std::chrono::duration_cast( + end_time - last_duration_update); + issuer_stats->add_sync_time(delta); + issuer_stats->inc_successful_validation(); } } catch (const std::exception &e) { - // Record failure if we have an issuer - if (!issuer.empty()) { + // Record failure (final duration update) + if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); + auto delta = std::chrono::duration_cast( + end_time - last_duration_update); + issuer_stats->add_sync_time(delta); + record_validation_error_stats(*issuer_stats, e); + } else if (!issuer.empty()) { + // Issuer known but stats not yet retrieved + auto &stats = + internal::MonitoringStats::instance().get_issuer_stats( + issuer); auto duration = - std::chrono::duration_cast>( - end_time - start_time); - record_validation_error(issuer, e, duration.count()); + std::chrono::duration_cast( + std::chrono::steady_clock::now() - start_time); + stats.add_sync_time(duration); + record_validation_error_stats(stats, e); } throw; } @@ -544,11 +674,17 @@ class Validator { void verify(const jwt::decoded_jwt &jwt) { std::string issuer = ""; auto start_time = std::chrono::steady_clock::now(); + internal::IssuerStats *issuer_stats = nullptr; try { // Try to extract issuer for monitoring if (jwt.has_payload_claim("iss")) { issuer = jwt.get_issuer(); + // Record sync validation started and get stats reference + issuer_stats = + &internal::MonitoringStats::instance().get_issuer_stats( + issuer); + issuer_stats->inc_sync_validation_started(); } auto result = verify_async(jwt); @@ -557,22 +693,33 @@ class Validator { } // Record successful validation - if (!issuer.empty()) { + if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); auto duration = - std::chrono::duration_cast>( + std::chrono::duration_cast( end_time - start_time); - internal::MonitoringStats::instance().record_validation_success( - issuer, duration.count()); + issuer_stats->add_sync_time(duration); + issuer_stats->inc_successful_validation(); } } catch (const std::exception &e) { // Record failure if we have an issuer - if (!issuer.empty()) { + if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); auto duration = - std::chrono::duration_cast>( + std::chrono::duration_cast( end_time - start_time); - record_validation_error(issuer, e, duration.count()); + issuer_stats->add_sync_time(duration); + record_validation_error_stats(*issuer_stats, e); + } else if (!issuer.empty()) { + // Issuer known but stats not yet retrieved + auto &stats = + internal::MonitoringStats::instance().get_issuer_stats( + issuer); + auto duration = + std::chrono::duration_cast( + std::chrono::steady_clock::now() - start_time); + stats.add_sync_time(duration); + record_validation_error_stats(stats, e); } throw; } @@ -653,9 +800,12 @@ class Validator { status->m_jwt_string = jwt.get_token(); status->m_public_pem = public_pem; status->m_algorithm = algorithm; - // Start monitoring timing + // Start monitoring timing and record async validation started status->m_start_time = std::chrono::steady_clock::now(); status->m_monitoring_started = true; + auto &stats = internal::MonitoringStats::instance().get_issuer_stats( + jwt.get_issuer()); + stats.inc_async_validation_started(); return verify_async_continue(std::move(status)); } @@ -830,14 +980,16 @@ class Validator { } } - // Record successful validation - if (status->m_monitoring_started) { + // Record successful validation (only for async API, sync handles its own) + if (status->m_monitoring_started && !status->m_is_sync) { auto end_time = std::chrono::steady_clock::now(); auto duration = - std::chrono::duration_cast>( + std::chrono::duration_cast( end_time - status->m_start_time); - internal::MonitoringStats::instance().record_validation_success( - status->m_issuer, duration.count()); + auto &stats = internal::MonitoringStats::instance().get_issuer_stats( + status->m_issuer); + stats.inc_successful_validation(); + stats.add_async_time(duration); } std::unique_ptr result(new AsyncStatus()); @@ -967,21 +1119,16 @@ class Validator { /** * Helper method to record monitoring statistics for validation errors. + * This version operates on an IssuerStats reference and does NOT update + * time (caller is responsible for time tracking). */ - void record_validation_error(const std::string &issuer, - const std::exception &e, double duration_s) { - // Check exception type instead of string introspection - if (dynamic_cast(&e)) { - internal::MonitoringStats::instance().record_failed_issuer_lookup( - issuer); - } - + void record_validation_error_stats(internal::IssuerStats &stats, + const std::exception &e) { if (dynamic_cast(&e)) { - internal::MonitoringStats::instance().record_expired_token(issuer); + stats.inc_expired_token(); } - internal::MonitoringStats::instance().record_validation_failure( - issuer, duration_s); + stats.inc_unsuccessful_validation(); } bool m_validate_all_claims{true}; diff --git a/src/scitokens_monitoring.cpp b/src/scitokens_monitoring.cpp index 9183033..be82df0 100644 --- a/src/scitokens_monitoring.cpp +++ b/src/scitokens_monitoring.cpp @@ -16,33 +16,8 @@ MonitoringStats &MonitoringStats::instance() { return instance; } -void MonitoringStats::record_validation_success(const std::string &issuer, - double duration_s) { - std::lock_guard lock(m_mutex); - auto &stats = m_issuer_stats[issuer]; - stats.successful_validations++; - // Add to the total time (accumulate across all validations) - // No atomic needed - protected by mutex - stats.total_time_s += duration_s; -} - -void MonitoringStats::record_validation_failure(const std::string &issuer, - double duration_s) { - std::lock_guard lock(m_mutex); - auto &stats = m_issuer_stats[issuer]; - stats.unsuccessful_validations++; - // Add to the total time (accumulate across all validations) - // No atomic needed - protected by mutex - stats.total_time_s += duration_s; -} - -void MonitoringStats::record_expired_token(const std::string &issuer) { - std::lock_guard lock(m_mutex); - auto &stats = m_issuer_stats[issuer]; - stats.expired_tokens++; -} - -void MonitoringStats::record_failed_issuer_lookup(const std::string &issuer) { +void MonitoringStats::record_failed_issuer_lookup(const std::string &issuer, + double duration_s) { std::lock_guard lock(m_mutex); // Limit the number of failed issuer entries to prevent resource exhaustion @@ -53,7 +28,9 @@ void MonitoringStats::record_failed_issuer_lookup(const std::string &issuer) { // Only track if we still have room or issuer is already tracked if (m_failed_issuer_lookups.size() < MAX_FAILED_ISSUERS || m_failed_issuer_lookups.find(issuer) != m_failed_issuer_lookups.end()) { - m_failed_issuer_lookups[issuer]++; + auto &stats = m_failed_issuer_lookups[issuer]; + stats.count++; + stats.total_time_s += duration_s; } } @@ -77,7 +54,7 @@ void MonitoringStats::prune_failed_issuers() { // Find the minimum count uint64_t min_count = UINT64_MAX; for (const auto &entry : m_failed_issuer_lookups) { - uint64_t count = entry.second; + uint64_t count = entry.second.count; if (count < min_count) { min_count = count; } @@ -86,7 +63,7 @@ void MonitoringStats::prune_failed_issuers() { // Remove all entries with the minimum count for (auto it = m_failed_issuer_lookups.begin(); it != m_failed_issuer_lookups.end();) { - if (it->second == min_count) { + if (it->second.count == min_count) { it = m_failed_issuer_lookups.erase(it); } else { ++it; @@ -112,8 +89,36 @@ std::string MonitoringStats::get_json() const { static_cast(stats.unsuccessful_validations.load())); issuer_obj["expired_tokens"] = picojson::value(static_cast(stats.expired_tokens.load())); + + // Validation started counters + issuer_obj["sync_validations_started"] = picojson::value( + static_cast(stats.sync_validations_started.load())); + issuer_obj["async_validations_started"] = picojson::value( + static_cast(stats.async_validations_started.load())); + + // Duration tracking + issuer_obj["sync_total_time_s"] = + picojson::value(stats.get_sync_time_s()); + issuer_obj["async_total_time_s"] = + picojson::value(stats.get_async_time_s()); issuer_obj["total_validation_time_s"] = - picojson::value(stats.total_time_s); + picojson::value(stats.get_total_time_s()); + + // Web lookup statistics + issuer_obj["successful_key_lookups"] = picojson::value( + static_cast(stats.successful_key_lookups.load())); + issuer_obj["failed_key_lookups"] = picojson::value( + static_cast(stats.failed_key_lookups.load())); + issuer_obj["failed_key_lookup_time_s"] = + picojson::value(stats.get_failed_key_lookup_time_s()); + + // Key refresh statistics + issuer_obj["expired_keys"] = + picojson::value(static_cast(stats.expired_keys.load())); + issuer_obj["failed_refreshes"] = + picojson::value(static_cast(stats.failed_refreshes.load())); + issuer_obj["stale_key_uses"] = + picojson::value(static_cast(stats.stale_key_uses.load())); std::string sanitized_issuer = sanitize_issuer_for_json(issuer); issuers_obj[sanitized_issuer] = picojson::value(issuer_obj); @@ -121,14 +126,18 @@ std::string MonitoringStats::get_json() const { root["issuers"] = picojson::value(issuers_obj); - // Add failed issuer lookups + // Add failed issuer lookups with duration if (!m_failed_issuer_lookups.empty()) { picojson::object failed_obj; for (const auto &entry : m_failed_issuer_lookups) { std::string sanitized_issuer = sanitize_issuer_for_json(entry.first); - failed_obj[sanitized_issuer] = - picojson::value(static_cast(entry.second)); + picojson::object lookup_stats; + lookup_stats["count"] = + picojson::value(static_cast(entry.second.count)); + lookup_stats["total_time_s"] = + picojson::value(entry.second.total_time_s); + failed_obj[sanitized_issuer] = picojson::value(lookup_stats); } root["failed_issuer_lookups"] = picojson::value(failed_obj); } diff --git a/test/integration_test.cpp b/test/integration_test.cpp index 5cf0047..cc8b176 100644 --- a/test/integration_test.cpp +++ b/test/integration_test.cpp @@ -24,7 +24,26 @@ class MonitoringStats { uint64_t successful_validations{0}; uint64_t unsuccessful_validations{0}; uint64_t expired_tokens{0}; + // Validation started counters + uint64_t sync_validations_started{0}; + uint64_t async_validations_started{0}; + // Duration tracking + double sync_total_time_s{0.0}; + double async_total_time_s{0.0}; double total_validation_time_s{0.0}; + // Key lookup statistics + uint64_t successful_key_lookups{0}; + uint64_t failed_key_lookups{0}; + double failed_key_lookup_time_s{0.0}; + // Key refresh statistics + uint64_t expired_keys{0}; + uint64_t failed_refreshes{0}; + uint64_t stale_key_uses{0}; + }; + + struct FailedIssuerLookup { + uint64_t count{0}; + double total_time_s{0.0}; }; bool parse(const std::string &json) { @@ -70,27 +89,102 @@ class MonitoringStats { static_cast(it->second.get()); } + // Validation started counters + it = stats_obj.find("sync_validations_started"); + if (it != stats_obj.end() && it->second.is()) { + stats.sync_validations_started = + static_cast(it->second.get()); + } + + it = stats_obj.find("async_validations_started"); + if (it != stats_obj.end() && it->second.is()) { + stats.async_validations_started = + static_cast(it->second.get()); + } + + // Duration tracking + it = stats_obj.find("sync_total_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.sync_total_time_s = it->second.get(); + } + + it = stats_obj.find("async_total_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.async_total_time_s = it->second.get(); + } + it = stats_obj.find("total_validation_time_s"); if (it != stats_obj.end() && it->second.is()) { stats.total_validation_time_s = it->second.get(); } + // Key lookup statistics + it = stats_obj.find("successful_key_lookups"); + if (it != stats_obj.end() && it->second.is()) { + stats.successful_key_lookups = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_key_lookups"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_key_lookups = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_key_lookup_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_key_lookup_time_s = + it->second.get(); + } + + // Key refresh statistics + it = stats_obj.find("expired_keys"); + if (it != stats_obj.end() && it->second.is()) { + stats.expired_keys = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_refreshes"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_refreshes = + static_cast(it->second.get()); + } + + it = stats_obj.find("stale_key_uses"); + if (it != stats_obj.end() && it->second.is()) { + stats.stale_key_uses = + static_cast(it->second.get()); + } + issuers_[issuer_entry.first] = stats; } } } - // Parse failed issuer lookups + // Parse failed issuer lookups (now has count and total_time_s) failed_issuer_lookups_.clear(); auto failed_it = root_obj.find("failed_issuer_lookups"); if (failed_it != root_obj.end() && failed_it->second.is()) { auto &failed_obj = failed_it->second.get(); for (const auto &entry : failed_obj) { - if (entry.second.is()) { - failed_issuer_lookups_[entry.first] = - static_cast(entry.second.get()); + if (entry.second.is()) { + FailedIssuerLookup lookup; + auto &lookup_obj = entry.second.get(); + + auto it = lookup_obj.find("count"); + if (it != lookup_obj.end() && it->second.is()) { + lookup.count = + static_cast(it->second.get()); + } + + it = lookup_obj.find("total_time_s"); + if (it != lookup_obj.end() && it->second.is()) { + lookup.total_time_s = it->second.get(); + } + + failed_issuer_lookups_[entry.first] = lookup; } } } @@ -106,12 +200,20 @@ class MonitoringStats { return IssuerStats{}; } - uint64_t getFailedLookupCount(const std::string &issuer) const { + FailedIssuerLookup getFailedLookup(const std::string &issuer) const { auto it = failed_issuer_lookups_.find(issuer); if (it != failed_issuer_lookups_.end()) { return it->second; } - return 0; + return FailedIssuerLookup{}; + } + + uint64_t getFailedLookupCount(const std::string &issuer) const { + return getFailedLookup(issuer).count; + } + + double getFailedLookupTime(const std::string &issuer) const { + return getFailedLookup(issuer).total_time_s; } size_t getIssuerCount() const { return issuers_.size(); } @@ -122,7 +224,7 @@ class MonitoringStats { private: std::map issuers_; - std::map failed_issuer_lookups_; + std::map failed_issuer_lookups_; }; // Helper to get current monitoring stats diff --git a/test/monitoring_test.cpp b/test/monitoring_test.cpp index 7d07fb7..abc8add 100644 --- a/test/monitoring_test.cpp +++ b/test/monitoring_test.cpp @@ -29,7 +29,26 @@ class MonitoringStats { uint64_t successful_validations{0}; uint64_t unsuccessful_validations{0}; uint64_t expired_tokens{0}; + // Validation started counters + uint64_t sync_validations_started{0}; + uint64_t async_validations_started{0}; + // Duration tracking + double sync_total_time_s{0.0}; + double async_total_time_s{0.0}; double total_validation_time_s{0.0}; + // Key lookup statistics + uint64_t successful_key_lookups{0}; + uint64_t failed_key_lookups{0}; + double failed_key_lookup_time_s{0.0}; + // Key refresh statistics + uint64_t expired_keys{0}; + uint64_t failed_refreshes{0}; + uint64_t stale_key_uses{0}; + }; + + struct FailedIssuerLookup { + uint64_t count{0}; + double total_time_s{0.0}; }; bool parse(const std::string &json) { @@ -75,27 +94,102 @@ class MonitoringStats { static_cast(it->second.get()); } + // Validation started counters + it = stats_obj.find("sync_validations_started"); + if (it != stats_obj.end() && it->second.is()) { + stats.sync_validations_started = + static_cast(it->second.get()); + } + + it = stats_obj.find("async_validations_started"); + if (it != stats_obj.end() && it->second.is()) { + stats.async_validations_started = + static_cast(it->second.get()); + } + + // Duration tracking + it = stats_obj.find("sync_total_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.sync_total_time_s = it->second.get(); + } + + it = stats_obj.find("async_total_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.async_total_time_s = it->second.get(); + } + it = stats_obj.find("total_validation_time_s"); if (it != stats_obj.end() && it->second.is()) { stats.total_validation_time_s = it->second.get(); } + // Key lookup statistics + it = stats_obj.find("successful_key_lookups"); + if (it != stats_obj.end() && it->second.is()) { + stats.successful_key_lookups = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_key_lookups"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_key_lookups = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_key_lookup_time_s"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_key_lookup_time_s = + it->second.get(); + } + + // Key refresh statistics + it = stats_obj.find("expired_keys"); + if (it != stats_obj.end() && it->second.is()) { + stats.expired_keys = + static_cast(it->second.get()); + } + + it = stats_obj.find("failed_refreshes"); + if (it != stats_obj.end() && it->second.is()) { + stats.failed_refreshes = + static_cast(it->second.get()); + } + + it = stats_obj.find("stale_key_uses"); + if (it != stats_obj.end() && it->second.is()) { + stats.stale_key_uses = + static_cast(it->second.get()); + } + issuers_[issuer_entry.first] = stats; } } } - // Parse failed issuer lookups + // Parse failed issuer lookups (now has count and total_time_s) failed_issuer_lookups_.clear(); auto failed_it = root_obj.find("failed_issuer_lookups"); if (failed_it != root_obj.end() && failed_it->second.is()) { auto &failed_obj = failed_it->second.get(); for (const auto &entry : failed_obj) { - if (entry.second.is()) { - failed_issuer_lookups_[entry.first] = - static_cast(entry.second.get()); + if (entry.second.is()) { + FailedIssuerLookup lookup; + auto &lookup_obj = entry.second.get(); + + auto it = lookup_obj.find("count"); + if (it != lookup_obj.end() && it->second.is()) { + lookup.count = + static_cast(it->second.get()); + } + + it = lookup_obj.find("total_time_s"); + if (it != lookup_obj.end() && it->second.is()) { + lookup.total_time_s = it->second.get(); + } + + failed_issuer_lookups_[entry.first] = lookup; } } } @@ -111,12 +205,20 @@ class MonitoringStats { return IssuerStats{}; } - uint64_t getFailedLookupCount(const std::string &issuer) const { + FailedIssuerLookup getFailedLookup(const std::string &issuer) const { auto it = failed_issuer_lookups_.find(issuer); if (it != failed_issuer_lookups_.end()) { return it->second; } - return 0; + return FailedIssuerLookup{}; + } + + uint64_t getFailedLookupCount(const std::string &issuer) const { + return getFailedLookup(issuer).count; + } + + double getFailedLookupTime(const std::string &issuer) const { + return getFailedLookup(issuer).total_time_s; } size_t getIssuerCount() const { return issuers_.size(); } @@ -127,7 +229,7 @@ class MonitoringStats { private: std::map issuers_; - std::map failed_issuer_lookups_; + std::map failed_issuer_lookups_; }; // Helper to get current monitoring stats From cb3e2e0c2b15d98f2fe2103dd3957e29efd1744c Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Wed, 10 Dec 2025 14:06:23 +0000 Subject: [PATCH 5/6] Add periodic monitoring file output with thread-safe configuration - Add monitoring.file config option to enable periodic JSON file output - Add monitoring.file_interval_s config option (default 60s) - Use atomic flag for fast-path check in hot verify() path - Use mutex-protected string for actual file path access - Write to temp file then atomic rename for safe updates - Silently ignore write errors (best-effort monitoring) - Integrate with existing scitoken_config_* API instead of dedicated functions - Add tests for configuration and file output --- src/scitokens.cpp | 48 ++++++++++++++++++ src/scitokens.h | 22 +++++++++ src/scitokens_internal.h | 35 +++++++++++++ src/scitokens_monitoring.cpp | 73 +++++++++++++++++++++++++++ test/integration_test.cpp | 92 ++++++++++++++++++++++++++++++++++ test/monitoring_test.cpp | 96 ++++++++++++++++++++++++++++++++++++ 6 files changed, 366 insertions(+) diff --git a/src/scitokens.cpp b/src/scitokens.cpp index 711f4c0..a844086 100644 --- a/src/scitokens.cpp +++ b/src/scitokens.cpp @@ -20,6 +20,34 @@ std::shared_ptr configurer::Configuration::m_cache_home = std::shared_ptr configurer::Configuration::m_tls_ca_file = std::make_shared(""); +// Monitoring file config (empty string means disabled) +// Protected by mutex; atomic flag for fast-path check +std::string configurer::Configuration::m_monitoring_file; +std::mutex configurer::Configuration::m_monitoring_file_mutex; +std::atomic configurer::Configuration::m_monitoring_file_configured{false}; +std::atomic_int configurer::Configuration::m_monitoring_file_interval{60}; + +void configurer::Configuration::set_monitoring_file(const std::string &path) { + std::lock_guard lock(m_monitoring_file_mutex); + m_monitoring_file = path; + // Update the atomic flag after setting the string + m_monitoring_file_configured.store(!path.empty(), + std::memory_order_release); +} + +std::string configurer::Configuration::get_monitoring_file() { + std::lock_guard lock(m_monitoring_file_mutex); + return m_monitoring_file; +} + +void configurer::Configuration::set_monitoring_file_interval(int seconds) { + m_monitoring_file_interval = seconds; +} + +int configurer::Configuration::get_monitoring_file_interval() { + return m_monitoring_file_interval; +} + SciTokenKey scitoken_key_create(const char *key_id, const char *alg, const char *public_contents, const char *private_contents, char **err_msg) { @@ -1026,6 +1054,17 @@ int scitoken_config_set_int(const char *key, int value, char **err_msg) { return 0; } + else if (_key == "monitoring.file_interval_s") { + if (value < 0) { + if (err_msg) { + *err_msg = strdup("Interval cannot be negative."); + } + return -1; + } + configurer::Configuration::set_monitoring_file_interval(value); + return 0; + } + else { if (err_msg) { *err_msg = strdup("Key not recognized."); @@ -1055,6 +1094,10 @@ int scitoken_config_get_int(const char *key, char **err_msg) { return configurer::Configuration::get_expiry_delta(); } + else if (_key == "monitoring.file_interval_s") { + return configurer::Configuration::get_monitoring_file_interval(); + } + else { if (err_msg) { *err_msg = strdup("Key not recognized."); @@ -1084,6 +1127,9 @@ int scitoken_config_set_str(const char *key, const char *value, } else if (_key == "tls.ca_file") { configurer::Configuration::set_tls_ca_file(value ? std::string(value) : ""); + } else if (_key == "monitoring.file") { + configurer::Configuration::set_monitoring_file(value ? std::string(value) + : ""); } else { if (err_msg) { *err_msg = strdup("Key not recognized."); @@ -1106,6 +1152,8 @@ int scitoken_config_get_str(const char *key, char **output, char **err_msg) { *output = strdup(configurer::Configuration::get_cache_home().c_str()); } else if (_key == "tls.ca_file") { *output = strdup(configurer::Configuration::get_tls_ca_file().c_str()); + } else if (_key == "monitoring.file") { + *output = strdup(configurer::Configuration::get_monitoring_file().c_str()); } else { diff --git a/src/scitokens.h b/src/scitokens.h index 7970043..173ac72 100644 --- a/src/scitokens.h +++ b/src/scitokens.h @@ -302,6 +302,11 @@ int config_set_int(const char *key, int value, char **err_msg); * Takes in key/value pairs and assigns the input value to whatever * configuration variable is indicated by the key. * Returns 0 on success, and non-zero for invalid keys or values. + * + * Supported keys: + * - "keycache.update_interval_s": Interval between key cache updates (seconds) + * - "keycache.expiration_interval_s": Key cache expiration time (seconds) + * - "monitoring.file_interval_s": Interval between monitoring file writes (seconds, default 60) */ int scitoken_config_set_int(const char *key, int value, char **err_msg); @@ -313,12 +318,24 @@ int config_get_int(const char *key, char **err_msg); * Returns the value associated with the supplied input key on success, and -1 * on failure. This assumes there are no keys for which a negative return value * is permissible. + * + * Supported keys: + * - "keycache.update_interval_s": Interval between key cache updates (seconds) + * - "keycache.expiration_interval_s": Key cache expiration time (seconds) + * - "monitoring.file_interval_s": Interval between monitoring file writes (seconds, default 60) */ int scitoken_config_get_int(const char *key, char **err_msg); /** * Set current scitokens str parameters. * Returns 0 on success, nonzero on failure + * + * Supported keys: + * - "keycache.cache_home": Directory for the key cache + * - "tls.ca_file": Path to TLS CA certificate file + * - "monitoring.file": Path to write monitoring JSON (empty to disable, default disabled) + * When enabled, monitoring stats are written periodically during verify() + * calls. The write interval is controlled by "monitoring.file_interval_s". */ int scitoken_config_set_str(const char *key, const char *value, char **err_msg); @@ -326,6 +343,11 @@ int scitoken_config_set_str(const char *key, const char *value, char **err_msg); * Get current scitokens str parameters. * Returns 0 on success, nonzero on failure, and populates the value associated * with the input key to output. + * + * Supported keys: + * - "keycache.cache_home": Directory for the key cache + * - "tls.ca_file": Path to TLS CA certificate file + * - "monitoring.file": Path to write monitoring JSON (empty if disabled) */ int scitoken_config_get_str(const char *key, char **output, char **err_msg); diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index 6244671..493e8a5 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -50,11 +50,25 @@ class Configuration { static void set_tls_ca_file(const std::string ca_file); static std::string get_tls_ca_file(); + // Monitoring file configuration + static void set_monitoring_file(const std::string &path); + static std::string get_monitoring_file(); + static void set_monitoring_file_interval(int seconds); + static int get_monitoring_file_interval(); + // Fast-path check: returns true if monitoring file might be configured + static bool is_monitoring_file_configured() { + return m_monitoring_file_configured.load(std::memory_order_relaxed); + } + private: static std::atomic_int m_next_update_delta; static std::atomic_int m_expiry_delta; static std::shared_ptr m_cache_home; static std::shared_ptr m_tls_ca_file; + static std::string m_monitoring_file; + static std::mutex m_monitoring_file_mutex; + static std::atomic m_monitoring_file_configured; // Fast-path flag + static std::atomic_int m_monitoring_file_interval; // In seconds, default 60 // static bool check_dir(const std::string dir_path); static std::pair mkdir_and_parents_if_needed(const std::string dir_path); @@ -240,6 +254,16 @@ class MonitoringStats { std::string get_json() const; void reset(); + /** + * Check if the monitoring file should be written and write it if so. + * This method is thread-safe and uses relaxed atomic operations for + * the fast path (checking if write is needed). Only one thread will + * actually perform the write. + * + * Does not throw exceptions - file write errors are silently ignored. + */ + void maybe_write_monitoring_file() noexcept; + private: MonitoringStats() = default; ~MonitoringStats() = default; @@ -253,8 +277,13 @@ class MonitoringStats { std::unordered_map m_issuer_stats; std::unordered_map m_failed_issuer_lookups; + // Atomic timestamp for last monitoring file write (seconds since epoch) + // Uses relaxed memory ordering for fast-path checks + std::atomic m_last_file_write_time{0}; + std::string sanitize_issuer_for_json(const std::string &issuer) const; void prune_failed_issuers(); + void write_monitoring_file_impl() noexcept; }; } // namespace internal @@ -584,6 +613,9 @@ class Validator { } void verify(const SciToken &scitoken, time_t expiry_time) { + // Check if monitoring file should be written (fast-path, relaxed atomic) + internal::MonitoringStats::instance().maybe_write_monitoring_file(); + std::string issuer = ""; auto start_time = std::chrono::steady_clock::now(); auto last_duration_update = start_time; @@ -672,6 +704,9 @@ class Validator { } void verify(const jwt::decoded_jwt &jwt) { + // Check if monitoring file should be written (fast-path, relaxed atomic) + internal::MonitoringStats::instance().maybe_write_monitoring_file(); + std::string issuer = ""; auto start_time = std::chrono::steady_clock::now(); internal::IssuerStats *issuer_stats = nullptr; diff --git a/src/scitokens_monitoring.cpp b/src/scitokens_monitoring.cpp index be82df0..ff5c1d1 100644 --- a/src/scitokens_monitoring.cpp +++ b/src/scitokens_monitoring.cpp @@ -1,6 +1,7 @@ #include "scitokens_internal.h" #include #include +#include #include #ifndef PICOJSON_USE_INT64 @@ -151,5 +152,77 @@ void MonitoringStats::reset() { m_failed_issuer_lookups.clear(); } +void MonitoringStats::maybe_write_monitoring_file() noexcept { + try { + // Fast path: check atomic flag first (relaxed load, no mutex) + if (!configurer::Configuration::is_monitoring_file_configured()) { + return; + } + + // Get current time and interval (relaxed loads for fast path) + auto now = std::chrono::steady_clock::now(); + auto now_seconds = std::chrono::duration_cast( + now.time_since_epoch()) + .count(); + int64_t last_write = + m_last_file_write_time.load(std::memory_order_relaxed); + int interval = configurer::Configuration::get_monitoring_file_interval(); + + // Check if enough time has passed since last write + if (now_seconds - last_write < interval) { + return; + } + + // Try to atomically claim the write (compare-and-swap) + // Only one thread will succeed in updating the timestamp + if (!m_last_file_write_time.compare_exchange_strong( + last_write, now_seconds, std::memory_order_acq_rel, + std::memory_order_relaxed)) { + // Another thread beat us to it, they will do the write + return; + } + + // We successfully claimed the write, do it + write_monitoring_file_impl(); + } catch (...) { + // Silently ignore any errors - this is best-effort + } +} + +void MonitoringStats::write_monitoring_file_impl() noexcept { + try { + std::string monitoring_file = + configurer::Configuration::get_monitoring_file(); + if (monitoring_file.empty()) { + return; + } + + // Get the JSON content + std::string json_content = get_json(); + + // Write to a temporary file first, then rename for atomicity + std::string tmp_file = monitoring_file + ".tmp"; + + { + std::ofstream ofs(tmp_file, std::ios::out | std::ios::trunc); + if (!ofs) { + return; // Cannot open file, silently fail + } + ofs << json_content; + if (!ofs) { + return; // Write failed, silently fail + } + } // Close file before rename + + // Atomic rename (on POSIX systems) + if (std::rename(tmp_file.c_str(), monitoring_file.c_str()) != 0) { + // Rename failed, try to clean up temp file + std::remove(tmp_file.c_str()); + } + } catch (...) { + // Silently ignore any errors - this is best-effort + } +} + } // namespace internal } // namespace scitokens diff --git a/test/integration_test.cpp b/test/integration_test.cpp index cc8b176..a04549f 100644 --- a/test/integration_test.cpp +++ b/test/integration_test.cpp @@ -1014,6 +1014,98 @@ TEST_F(IntegrationTest, MonitoringDurationTracking) { << issuer_stats.total_validation_time_s << std::endl; } +// Test monitoring file output during token verification +TEST_F(IntegrationTest, MonitoringFileOutput) { + char *err_msg = nullptr; + + // Set up a test file path and zero interval for immediate write + std::string test_file = "/tmp/scitokens_monitoring_integration_" + + std::to_string(time(nullptr)) + ".json"; + scitoken_config_set_str("monitoring.file", test_file.c_str(), &err_msg); + scitoken_config_set_int("monitoring.file_interval_s", 0, &err_msg); + + // Clean up any existing file + std::remove(test_file.c_str()); + + // Reset monitoring stats + scitoken_reset_monitoring_stats(&err_msg); + + // Create and verify a token (this should trigger file write) + // Use test-key-1 to match the key ID in the JWKS server + SciTokenKey key = scitoken_key_create("test-key-1", "ES256", + public_key_.c_str(), + private_key_.c_str(), &err_msg); + ASSERT_TRUE(key != nullptr); + std::unique_ptr key_ptr( + key, scitoken_key_destroy); + + SciToken token = scitoken_create(key); + ASSERT_TRUE(token != nullptr); + std::unique_ptr token_ptr( + token, scitoken_destroy); + + scitoken_set_claim_string(token, "iss", issuer_url_.c_str(), &err_msg); + scitoken_set_claim_string(token, "sub", "test-user", &err_msg); + scitoken_set_claim_string(token, "scope", "read:/test", &err_msg); + + char *token_value = nullptr; + int rv = scitoken_serialize(token, &token_value, &err_msg); + ASSERT_EQ(rv, 0); + std::unique_ptr token_value_ptr(token_value, free); + + // Verify the token - this should trigger monitoring file write + std::unique_ptr verify_token( + scitoken_create(nullptr), scitoken_destroy); + ASSERT_TRUE(verify_token.get() != nullptr); + + rv = scitoken_deserialize_v2(token_value, verify_token.get(), nullptr, + &err_msg); + if (rv != 0 && err_msg) { + std::cerr << "Token verification error: " << err_msg << std::endl; + } + ASSERT_EQ(rv, 0) << "Token verification should succeed"; + if (err_msg) { + free(err_msg); + err_msg = nullptr; + } + + // Check that the monitoring file was created + std::ifstream file(test_file); + EXPECT_TRUE(file.good()) << "Monitoring file should have been created at " + << test_file; + + if (file.good()) { + // Read and parse the file + std::stringstream buffer; + buffer << file.rdbuf(); + std::string content = buffer.str(); + + EXPECT_FALSE(content.empty()) << "Monitoring file should not be empty"; + + // Try to parse it as JSON + picojson::value root; + std::string parse_err = picojson::parse(root, content); + EXPECT_TRUE(parse_err.empty()) + << "Monitoring file should contain valid JSON: " << parse_err; + + if (parse_err.empty()) { + // Verify it has the expected structure + EXPECT_TRUE(root.is()); + auto &root_obj = root.get(); + EXPECT_TRUE(root_obj.find("issuers") != root_obj.end()) + << "Monitoring JSON should have 'issuers' key"; + } + + std::cout << "Monitoring file content:" << std::endl; + std::cout << content << std::endl; + } + + // Clean up + scitoken_config_set_str("monitoring.file", "", &err_msg); + scitoken_config_set_int("monitoring.file_interval_s", 60, &err_msg); + std::remove(test_file.c_str()); +} + } // namespace int main(int argc, char **argv) { diff --git a/test/monitoring_test.cpp b/test/monitoring_test.cpp index abc8add..321b9d4 100644 --- a/test/monitoring_test.cpp +++ b/test/monitoring_test.cpp @@ -338,6 +338,102 @@ TEST_F(MonitoringTest, DDoSProtection) { free(err_msg); } +// Test monitoring file configuration API +TEST_F(MonitoringTest, MonitoringFileConfiguration) { + char *err_msg = nullptr; + char *path = nullptr; + int interval = 0; + + // Initially disabled (empty string) + int rv = scitoken_config_get_str("monitoring.file", &path, &err_msg); + EXPECT_EQ(rv, 0); + EXPECT_NE(path, nullptr); + EXPECT_STREQ(path, ""); + free(path); + path = nullptr; + + // Default interval should be 60 seconds + interval = scitoken_config_get_int("monitoring.file_interval_s", &err_msg); + EXPECT_EQ(interval, 60); + + // Set a monitoring file path + rv = scitoken_config_set_str("monitoring.file", + "/tmp/scitokens_test_monitoring.json", + &err_msg); + EXPECT_EQ(rv, 0); + + rv = scitoken_config_get_str("monitoring.file", &path, &err_msg); + EXPECT_EQ(rv, 0); + EXPECT_NE(path, nullptr); + EXPECT_STREQ(path, "/tmp/scitokens_test_monitoring.json"); + free(path); + path = nullptr; + + // Set a custom interval + rv = scitoken_config_set_int("monitoring.file_interval_s", 30, &err_msg); + EXPECT_EQ(rv, 0); + + interval = scitoken_config_get_int("monitoring.file_interval_s", &err_msg); + EXPECT_EQ(interval, 30); + + // Disable by setting to empty string + rv = scitoken_config_set_str("monitoring.file", "", &err_msg); + EXPECT_EQ(rv, 0); + + rv = scitoken_config_get_str("monitoring.file", &path, &err_msg); + EXPECT_EQ(rv, 0); + EXPECT_NE(path, nullptr); + EXPECT_STREQ(path, ""); + free(path); + path = nullptr; + + // Disable by setting to nullptr + rv = scitoken_config_set_str("monitoring.file", nullptr, &err_msg); + EXPECT_EQ(rv, 0); + + rv = scitoken_config_get_str("monitoring.file", &path, &err_msg); + EXPECT_EQ(rv, 0); + EXPECT_NE(path, nullptr); + EXPECT_STREQ(path, ""); + free(path); + path = nullptr; + + // Reset interval to default for other tests + scitoken_config_set_int("monitoring.file_interval_s", 60, &err_msg); +} + +// Test monitoring file write with zero interval (immediate write) +TEST_F(MonitoringTest, MonitoringFileWrite) { + char *err_msg = nullptr; + + // Set up a test file path and zero interval for immediate write + std::string test_file = "/tmp/scitokens_monitoring_test_" + + std::to_string(time(nullptr)) + ".json"; + scitoken_config_set_str("monitoring.file", test_file.c_str(), &err_msg); + scitoken_config_set_int("monitoring.file_interval_s", 0, &err_msg); + + // Clean up any existing file + std::remove(test_file.c_str()); + + // Reset stats and record something + scitoken_reset_monitoring_stats(&err_msg); + + // The maybe_write_monitoring_file is called during verify(), but we can't + // easily trigger that without a valid token/issuer. However, we can test + // the configuration API works and that files aren't written when disabled. + + // Verify file doesn't exist yet (nothing to trigger write) + FILE *f = fopen(test_file.c_str(), "r"); + EXPECT_EQ(f, nullptr); // File should not exist + + // Disable monitoring file + scitoken_config_set_str("monitoring.file", "", &err_msg); + scitoken_config_set_int("monitoring.file_interval_s", 60, &err_msg); + + // Clean up test file if it was created + std::remove(test_file.c_str()); +} + } // namespace int main(int argc, char **argv) { From 87848a04a92126babd5405b29eb8a93521df4bbe Mon Sep 17 00:00:00 2001 From: Brian P Bockelman Date: Wed, 10 Dec 2025 14:11:31 +0000 Subject: [PATCH 6/6] Apply clang-format to source files --- src/scitokens.cpp | 10 +++++---- src/scitokens.h | 16 +++++++++----- src/scitokens_internal.h | 43 ++++++++++++++++++++++-------------- src/scitokens_monitoring.cpp | 3 ++- src/verify.cpp | 3 ++- test/integration_test.cpp | 10 ++++----- test/monitoring_test.cpp | 5 ++--- 7 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/scitokens.cpp b/src/scitokens.cpp index a844086..1d53f16 100644 --- a/src/scitokens.cpp +++ b/src/scitokens.cpp @@ -24,7 +24,8 @@ std::shared_ptr configurer::Configuration::m_tls_ca_file = // Protected by mutex; atomic flag for fast-path check std::string configurer::Configuration::m_monitoring_file; std::mutex configurer::Configuration::m_monitoring_file_mutex; -std::atomic configurer::Configuration::m_monitoring_file_configured{false}; +std::atomic configurer::Configuration::m_monitoring_file_configured{ + false}; std::atomic_int configurer::Configuration::m_monitoring_file_interval{60}; void configurer::Configuration::set_monitoring_file(const std::string &path) { @@ -1128,8 +1129,8 @@ int scitoken_config_set_str(const char *key, const char *value, configurer::Configuration::set_tls_ca_file(value ? std::string(value) : ""); } else if (_key == "monitoring.file") { - configurer::Configuration::set_monitoring_file(value ? std::string(value) - : ""); + configurer::Configuration::set_monitoring_file( + value ? std::string(value) : ""); } else { if (err_msg) { *err_msg = strdup("Key not recognized."); @@ -1153,7 +1154,8 @@ int scitoken_config_get_str(const char *key, char **output, char **err_msg) { } else if (_key == "tls.ca_file") { *output = strdup(configurer::Configuration::get_tls_ca_file().c_str()); } else if (_key == "monitoring.file") { - *output = strdup(configurer::Configuration::get_monitoring_file().c_str()); + *output = + strdup(configurer::Configuration::get_monitoring_file().c_str()); } else { diff --git a/src/scitokens.h b/src/scitokens.h index 173ac72..cdf3953 100644 --- a/src/scitokens.h +++ b/src/scitokens.h @@ -306,7 +306,8 @@ int config_set_int(const char *key, int value, char **err_msg); * Supported keys: * - "keycache.update_interval_s": Interval between key cache updates (seconds) * - "keycache.expiration_interval_s": Key cache expiration time (seconds) - * - "monitoring.file_interval_s": Interval between monitoring file writes (seconds, default 60) + * - "monitoring.file_interval_s": Interval between monitoring file writes + * (seconds, default 60) */ int scitoken_config_set_int(const char *key, int value, char **err_msg); @@ -322,7 +323,8 @@ int config_get_int(const char *key, char **err_msg); * Supported keys: * - "keycache.update_interval_s": Interval between key cache updates (seconds) * - "keycache.expiration_interval_s": Key cache expiration time (seconds) - * - "monitoring.file_interval_s": Interval between monitoring file writes (seconds, default 60) + * - "monitoring.file_interval_s": Interval between monitoring file writes + * (seconds, default 60) */ int scitoken_config_get_int(const char *key, char **err_msg); @@ -333,9 +335,10 @@ int scitoken_config_get_int(const char *key, char **err_msg); * Supported keys: * - "keycache.cache_home": Directory for the key cache * - "tls.ca_file": Path to TLS CA certificate file - * - "monitoring.file": Path to write monitoring JSON (empty to disable, default disabled) - * When enabled, monitoring stats are written periodically during verify() - * calls. The write interval is controlled by "monitoring.file_interval_s". + * - "monitoring.file": Path to write monitoring JSON (empty to disable, default + * disabled) When enabled, monitoring stats are written periodically during + * verify() calls. The write interval is controlled by + * "monitoring.file_interval_s". */ int scitoken_config_set_str(const char *key, const char *value, char **err_msg); @@ -361,7 +364,8 @@ int scitoken_config_get_str(const char *key, char **output, char **err_msg); * - expired_tokens: count of expired tokens encountered * - sync_validations_started: count of validations started via blocking API * - async_validations_started: count of validations started via async API - * - sync_total_time_s: time spent in blocking verify() calls (updated every 50ms) + * - sync_total_time_s: time spent in blocking verify() calls (updated every + * 50ms) * - async_total_time_s: time spent in async validations (updated on completion) * - total_validation_time_s: sum of sync and async time * - successful_key_lookups: count of successful JWKS web refreshes diff --git a/src/scitokens_internal.h b/src/scitokens_internal.h index 493e8a5..44bf2a7 100644 --- a/src/scitokens_internal.h +++ b/src/scitokens_internal.h @@ -68,7 +68,7 @@ class Configuration { static std::string m_monitoring_file; static std::mutex m_monitoring_file_mutex; static std::atomic m_monitoring_file_configured; // Fast-path flag - static std::atomic_int m_monitoring_file_interval; // In seconds, default 60 + static std::atomic_int m_monitoring_file_interval; // In seconds, default 60 // static bool check_dir(const std::string dir_path); static std::pair mkdir_and_parents_if_needed(const std::string dir_path); @@ -140,8 +140,10 @@ struct IssuerStats { std::atomic expired_tokens{0}; // Validation started counters (separate from results) - std::atomic sync_validations_started{0}; // Started via blocking verify() - std::atomic async_validations_started{0}; // Started via verify_async() + std::atomic sync_validations_started{ + 0}; // Started via blocking verify() + std::atomic async_validations_started{ + 0}; // Started via verify_async() // Duration tracking (nanoseconds) // sync_total_time_ns is updated periodically during blocking verify() @@ -417,7 +419,8 @@ class AsyncStatus { std::string m_algorithm; std::chrono::steady_clock::time_point m_start_time; bool m_monitoring_started{false}; - bool m_is_sync{false}; // True if called from blocking verify(), false for pure async + bool m_is_sync{ + false}; // True if called from blocking verify(), false for pure async struct timeval get_timeout_val(time_t expiry_time) const { auto now = time(NULL); @@ -613,7 +616,8 @@ class Validator { } void verify(const SciToken &scitoken, time_t expiry_time) { - // Check if monitoring file should be written (fast-path, relaxed atomic) + // Check if monitoring file should be written (fast-path, relaxed + // atomic) internal::MonitoringStats::instance().maybe_write_monitoring_file(); std::string issuer = ""; @@ -653,8 +657,9 @@ class Validator { // Update duration periodically on each select return if (issuer_stats) { auto now = std::chrono::steady_clock::now(); - auto delta = std::chrono::duration_cast< - std::chrono::nanoseconds>(now - last_duration_update); + auto delta = + std::chrono::duration_cast( + now - last_duration_update); issuer_stats->add_sync_time(delta); last_duration_update = now; } @@ -664,7 +669,8 @@ class Validator { "Timeout when loading the OIDC metadata."); } - // Only continue if select returned due to I/O activity (not timeout) + // Only continue if select returned due to I/O activity (not + // timeout) if (select_result > 0) { result = verify_async_continue(std::move(result)); } @@ -675,8 +681,9 @@ class Validator { // Record successful validation (final duration update) if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); - auto delta = std::chrono::duration_cast( - end_time - last_duration_update); + auto delta = + std::chrono::duration_cast( + end_time - last_duration_update); issuer_stats->add_sync_time(delta); issuer_stats->inc_successful_validation(); } @@ -684,8 +691,9 @@ class Validator { // Record failure (final duration update) if (issuer_stats) { auto end_time = std::chrono::steady_clock::now(); - auto delta = std::chrono::duration_cast( - end_time - last_duration_update); + auto delta = + std::chrono::duration_cast( + end_time - last_duration_update); issuer_stats->add_sync_time(delta); record_validation_error_stats(*issuer_stats, e); } else if (!issuer.empty()) { @@ -704,7 +712,8 @@ class Validator { } void verify(const jwt::decoded_jwt &jwt) { - // Check if monitoring file should be written (fast-path, relaxed atomic) + // Check if monitoring file should be written (fast-path, relaxed + // atomic) internal::MonitoringStats::instance().maybe_write_monitoring_file(); std::string issuer = ""; @@ -1015,14 +1024,16 @@ class Validator { } } - // Record successful validation (only for async API, sync handles its own) + // Record successful validation (only for async API, sync handles its + // own) if (status->m_monitoring_started && !status->m_is_sync) { auto end_time = std::chrono::steady_clock::now(); auto duration = std::chrono::duration_cast( end_time - status->m_start_time); - auto &stats = internal::MonitoringStats::instance().get_issuer_stats( - status->m_issuer); + auto &stats = + internal::MonitoringStats::instance().get_issuer_stats( + status->m_issuer); stats.inc_successful_validation(); stats.add_async_time(duration); } diff --git a/src/scitokens_monitoring.cpp b/src/scitokens_monitoring.cpp index ff5c1d1..b264287 100644 --- a/src/scitokens_monitoring.cpp +++ b/src/scitokens_monitoring.cpp @@ -166,7 +166,8 @@ void MonitoringStats::maybe_write_monitoring_file() noexcept { .count(); int64_t last_write = m_last_file_write_time.load(std::memory_order_relaxed); - int interval = configurer::Configuration::get_monitoring_file_interval(); + int interval = + configurer::Configuration::get_monitoring_file_interval(); // Check if enough time has passed since last write if (now_seconds - last_write < interval) { diff --git a/src/verify.cpp b/src/verify.cpp index c65aaf9..5bc055e 100644 --- a/src/verify.cpp +++ b/src/verify.cpp @@ -99,7 +99,8 @@ int main(int argc, char *const *argv) { std::getline(std::cin, token); } if (token.empty()) { - fprintf(stderr, "%s: No token provided on stdin or command line.\n", argv[0]); + fprintf(stderr, "%s: No token provided on stdin or command line.\n", + argv[0]); fprintf(stderr, usage, argv[0]); return 1; } diff --git a/test/integration_test.cpp b/test/integration_test.cpp index a04549f..15922cc 100644 --- a/test/integration_test.cpp +++ b/test/integration_test.cpp @@ -1032,9 +1032,9 @@ TEST_F(IntegrationTest, MonitoringFileOutput) { // Create and verify a token (this should trigger file write) // Use test-key-1 to match the key ID in the JWKS server - SciTokenKey key = scitoken_key_create("test-key-1", "ES256", - public_key_.c_str(), - private_key_.c_str(), &err_msg); + SciTokenKey key = + scitoken_key_create("test-key-1", "ES256", public_key_.c_str(), + private_key_.c_str(), &err_msg); ASSERT_TRUE(key != nullptr); std::unique_ptr key_ptr( key, scitoken_key_destroy); @@ -1071,8 +1071,8 @@ TEST_F(IntegrationTest, MonitoringFileOutput) { // Check that the monitoring file was created std::ifstream file(test_file); - EXPECT_TRUE(file.good()) << "Monitoring file should have been created at " - << test_file; + EXPECT_TRUE(file.good()) + << "Monitoring file should have been created at " << test_file; if (file.good()) { // Read and parse the file diff --git a/test/monitoring_test.cpp b/test/monitoring_test.cpp index 321b9d4..c27c3b0 100644 --- a/test/monitoring_test.cpp +++ b/test/monitoring_test.cpp @@ -357,9 +357,8 @@ TEST_F(MonitoringTest, MonitoringFileConfiguration) { EXPECT_EQ(interval, 60); // Set a monitoring file path - rv = scitoken_config_set_str("monitoring.file", - "/tmp/scitokens_test_monitoring.json", - &err_msg); + rv = scitoken_config_set_str( + "monitoring.file", "/tmp/scitokens_test_monitoring.json", &err_msg); EXPECT_EQ(rv, 0); rv = scitoken_config_get_str("monitoring.file", &path, &err_msg);