Add negative cache for failed issuer lookups

src/scitokens_cache.cpp

@@ -209,6 +209,35 @@ bool scitokens::Validator::get_public_keys_from_db(const std::string issuer,
             return false;
         }
         auto keys_local = iter->second;
+
+        // Check if this is a negative cache entry (empty keys array)
+        if (keys_local.is<picojson::object>()) {
+            auto jwks_obj = keys_local.get<picojson::object>();
+            auto keys_iter = jwks_obj.find("keys");
+            if (keys_iter != jwks_obj.end() &&
+                keys_iter->second.is<picojson::array>()) {
+                auto keys_array = keys_iter->second.get<picojson::array>();
+                if (keys_array.empty()) {
+                    // Check if negative cache has expired
+                    iter = top_obj.find("expires");
+                    if (iter != top_obj.end() && iter->second.is<int64_t>()) {
+                        auto expiry = iter->second.get<int64_t>();
+                        if (now > expiry) {
+                            // Negative cache expired, remove and return false
+                            if (remove_issuer_entry(db, issuer, true) != 0) {
+                                return false;
+                            }
+                            sqlite3_close(db);
+                            return false;
+                        }
+                    }
+                    // Negative cache still valid - throw exception
+                    sqlite3_close(db);
+                    throw NegativeCacheHitException(issuer);
+                }
+            }
+        }
+
         iter = top_obj.find("expires");
         if (iter == top_obj.end() || !iter->second.is<int64_t>()) {
             if (remove_issuer_entry(db, issuer, true) != 0) {

src/scitokens_internal.cpp

@@ -921,8 +921,14 @@ std::string Validator::get_jwks(const std::string &issuer) {
     auto now = std::time(NULL);
     picojson::value jwks;
     int64_t next_update;
-    if (get_public_keys_from_db(issuer, now, jwks, next_update)) {
-        return jwks.serialize();
+    try {
+        if (get_public_keys_from_db(issuer, now, jwks, next_update)) {
+            return jwks.serialize();
+        }
+    } catch (const NegativeCacheHitException &) {
+        // Negative cache hit - return empty keys without incrementing counter
+        // (counter is incremented elsewhere for validation failures)
+        return std::string("{\"keys\": []}");
     }
     return std::string("{\"keys\": []}");
 }

src/scitokens_internal.h

@@ -264,6 +264,9 @@ struct IssuerStats {
     std::atomic<uint64_t> background_successful_refreshes{0};
     std::atomic<uint64_t> background_failed_refreshes{0};
 
+    // Negative cache statistics
+    std::atomic<uint64_t> negative_cache_hits{0};
+
     // Increment methods for atomic counters (use relaxed ordering for stats)
     void inc_successful_validation() {
         successful_validations.fetch_add(1, std::memory_order_relaxed);
@@ -301,6 +304,9 @@ struct IssuerStats {
     void inc_background_failed_refresh() {
         background_failed_refreshes.fetch_add(1, std::memory_order_relaxed);
     }
+    void inc_negative_cache_hit() {
+        negative_cache_hits.fetch_add(1, std::memory_order_relaxed);
+    }
 
     // Time setters that accept std::chrono::duration (use relaxed ordering)
     template <typename Rep, typename Period>
@@ -476,6 +482,14 @@ class InvalidIssuerException : public std::runtime_error {
     InvalidIssuerException(const std::string &msg) : std::runtime_error(msg) {}
 };
 
+class NegativeCacheHitException : public InvalidIssuerException {
+  public:
+    explicit NegativeCacheHitException(const std::string &issuer)
+        : InvalidIssuerException("Issuer is in negative cache (recently failed "
+                                 "to retrieve keys): " +
+                                 issuer) {}
+};
+
 class JsonException : public std::runtime_error {
   public:
     JsonException(const std::string &msg) : std::runtime_error(msg) {}
@@ -1350,6 +1364,8 @@ class Validator {
                                        const std::exception &e) {
         if (dynamic_cast<const TokenExpiredException *>(&e)) {
             stats.inc_expired_token();
+        } else if (dynamic_cast<const NegativeCacheHitException *>(&e)) {
+            stats.inc_negative_cache_hit();
         }
 
         stats.inc_unsuccessful_validation();

src/scitokens_monitoring.cpp

@@ -134,6 +134,11 @@ std::string MonitoringStats::get_json() const {
             static_cast<int64_t>(stats.background_failed_refreshes.load(
                 std::memory_order_relaxed)));
 
+        // Negative cache statistics
+        issuer_obj["negative_cache_hits"] =
+            picojson::value(static_cast<int64_t>(
+                stats.negative_cache_hits.load(std::memory_order_relaxed)));
+
         std::string sanitized_issuer = sanitize_issuer_for_json(issuer);
         issuers_obj[sanitized_issuer] = picojson::value(issuer_obj);
     }

test/integration_test.cpp

@@ -1,14 +1,17 @@
 #include "../src/scitokens.h"
+#include "test_utils.h"
 
 #include <atomic>
 #include <cmath>
+#include <cstdlib>
 #include <fstream>
 #include <gtest/gtest.h>
 #include <iostream>
 #include <map>
 #include <memory>
 #include <sstream>
 #include <string>
+#include <sys/stat.h>
 #include <thread>
 #include <unistd.h>
 #include <vector>
@@ -18,6 +21,8 @@
 #endif
 #include <picojson/picojson.h>
 
+using scitokens_test::SecureTempDir;
+
 namespace {
 
 // Helper class to parse monitoring JSON
@@ -1037,15 +1042,15 @@ TEST_F(IntegrationTest, MonitoringDurationTracking) {
 TEST_F(IntegrationTest, MonitoringFileOutput) {
     char *err_msg = nullptr;
 
+    // Create a secure temp directory for the monitoring file
+    SecureTempDir temp_dir("monitoring_test_");
+    ASSERT_TRUE(temp_dir.valid()) << "Failed to create temp directory";
+
     // 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";
+    std::string test_file = temp_dir.path() + "/monitoring.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);
 
@@ -1119,10 +1124,10 @@ TEST_F(IntegrationTest, MonitoringFileOutput) {
         std::cout << content << std::endl;
     }
 
-    // Clean up
+    // Clean up - disable monitoring file
     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());
+    // temp_dir destructor will clean up the directory and file
 }
 
 // =============================================================================
@@ -1347,13 +1352,14 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
 TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
     char *err_msg = nullptr;
 
-    // Use a unique cache directory to ensure no cached keys exist
+    // Use a unique secure cache directory to ensure no cached keys exist
     // This forces the code path where keys must be fetched from the server
-    std::string unique_cache_dir = "/tmp/scitokens_concurrent_test_" +
-                                   std::to_string(time(nullptr)) + "_" +
-                                   std::to_string(getpid());
+    SecureTempDir unique_cache("concurrent_test_");
+    ASSERT_TRUE(unique_cache.valid())
+        << "Failed to create temp cache directory";
+
     int rv = scitoken_config_set_str("keycache.cache_home",
-                                     unique_cache_dir.c_str(), &err_msg);
+                                     unique_cache.path().c_str(), &err_msg);
     ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
                      << (err_msg ? err_msg : "unknown");
     if (err_msg) {
@@ -1413,7 +1419,7 @@ TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
     auto initial_key_lookups =
         stats_before.getIssuerStats(issuer_url_).successful_key_lookups;
 
-    std::cout << "Using unique cache directory: " << unique_cache_dir
+    std::cout << "Using unique cache directory: " << unique_cache.path()
               << std::endl;
     std::cout << "Initial successful_validations: "
               << initial_successful_validations << std::endl;
@@ -1504,24 +1510,23 @@ TEST_F(IntegrationTest, ConcurrentNewIssuerLookup) {
     EXPECT_EQ(new_expired_keys, static_cast<uint64_t>(NUM_THREADS))
         << "All threads should enter the expired_keys code path";
 
-    // Cleanup: remove the temporary cache directory
-    std::string rm_cmd = "rm -rf " + unique_cache_dir;
-    (void)system(rm_cmd.c_str());
+    // unique_cache destructor will clean up the temporary cache directory
 }
 
 // Stress test: repeatedly deserialize a valid token across multiple threads
 // for a fixed duration and verify monitoring counters match actual counts
 TEST_F(IntegrationTest, StressTestValidToken) {
     char *err_msg = nullptr;
 
-    // Use a unique cache directory to ensure no cached keys exist from prior
-    // tests This forces fresh key lookup and prevents background refresh
-    // interference
-    std::string unique_cache_dir = "/tmp/scitokens_stress_valid_" +
-                                   std::to_string(time(nullptr)) + "_" +
-                                   std::to_string(getpid());
+    // Use a unique secure cache directory to ensure no cached keys exist from
+    // prior tests. This forces fresh key lookup and prevents background refresh
+    // interference.
+    SecureTempDir unique_cache("stress_valid_");
+    ASSERT_TRUE(unique_cache.valid())
+        << "Failed to create temp cache directory";
+
     int rv = scitoken_config_set_str("keycache.cache_home",
-                                     unique_cache_dir.c_str(), &err_msg);
+                                     unique_cache.path().c_str(), &err_msg);
     ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
                      << (err_msg ? err_msg : "unknown");
     if (err_msg) {
@@ -1564,7 +1569,6 @@ TEST_F(IntegrationTest, StressTestValidToken) {
         free(err_msg);
         err_msg = nullptr;
     }
-
     std::unique_ptr<void, decltype(&scitoken_destroy)> token(
         scitoken_create(key.get()), scitoken_destroy);
     ASSERT_TRUE(token.get() != nullptr);
@@ -1697,9 +1701,7 @@ TEST_F(IntegrationTest, StressTestValidToken) {
         << "Should have completed at least 100 validations in "
         << TEST_DURATION_MS << "ms";
 
-    // Cleanup: remove the temporary cache directory
-    std::string rm_cmd = "rm -rf " + unique_cache_dir;
-    (void)system(rm_cmd.c_str());
+    // unique_cache destructor will clean up the temporary cache directory
 }
 
 // Stress test: repeatedly deserialize a token with an invalid issuer (404)
@@ -1708,13 +1710,14 @@ TEST_F(IntegrationTest, StressTestValidToken) {
 TEST_F(IntegrationTest, StressTestInvalidIssuer) {
     char *err_msg = nullptr;
 
-    // Use a unique cache directory to ensure no cached keys exist from prior
-    // tests
-    std::string unique_cache_dir = "/tmp/scitokens_stress_invalid_" +
-                                   std::to_string(time(nullptr)) + "_" +
-                                   std::to_string(getpid());
+    // Use a unique secure cache directory to ensure no cached keys exist from
+    // prior tests
+    SecureTempDir unique_cache("stress_invalid_");
+    ASSERT_TRUE(unique_cache.valid())
+        << "Failed to create temp cache directory";
+
     int rv = scitoken_config_set_str("keycache.cache_home",
-                                     unique_cache_dir.c_str(), &err_msg);
+                                     unique_cache.path().c_str(), &err_msg);
     ASSERT_EQ(rv, 0) << "Failed to set cache_home: "
                      << (err_msg ? err_msg : "unknown");
     if (err_msg) {
@@ -1873,9 +1876,7 @@ TEST_F(IntegrationTest, StressTestInvalidIssuer) {
         << "Should have completed at least 100 validations in "
         << TEST_DURATION_MS << "ms";
 
-    // Cleanup: remove the temporary cache directory
-    std::string rm_cmd = "rm -rf " + unique_cache_dir;
-    (void)system(rm_cmd.c_str());
+    // unique_cache destructor will clean up the temporary cache directory
 }
 
 } // namespace