From a8f705268431f5072d837e902a87cc98a4c6ead8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 27 Aug 2026 20:40:03 +0000 Subject: [PATCH 1/6] Initial plan From 1fd55cc6b42eae91b80c945cf46c42180b5d727b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 27 Aug 2026 21:31:58 +0000 Subject: [PATCH 2/6] Retry failed JWT key refreshes Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- CHANGELOG.md | 1 + doc/build_apps/auth/jwt.rst | 2 +- src/node/jwt_key_auto_refresh.h | 180 +++++++++++++++++++++++++++++--- tests/jwt_test.py | 21 +++- 4 files changed, 184 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 126fdff584c8..aba7ffbe291c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Nodes from the previous service are now removed during disaster recovery instead of being retained as retired entries in `GET /node/network/nodes`, and `ledger_code.py` reports their code identities as removed (#8177). - Fixed an edge case where a follower could incorrectly commit to an abandoned fork while synchronising with the leader, causing it to become unavailable (#8172). +- JWT/JWK auto-refresh failures are now retried after the lesser of 5 seconds and the configured key refresh interval, with exponential backoff capped at that interval. (#8226, #3869) ## [7.0.12] diff --git a/doc/build_apps/auth/jwt.rst b/doc/build_apps/auth/jwt.rst index e800255677dc..34d17058b92e 100644 --- a/doc/build_apps/auth/jwt.rst +++ b/doc/build_apps/auth/jwt.rst @@ -107,7 +107,7 @@ Now the issuer can be created with auto-refresh enabled: .. note:: - The key refresh interval is set via the ``jwt.key_refresh_interval`` configuration entry, where the default is 30 min (1800 seconds). The maximum response body size accepted when fetching OpenID metadata and JWKS is set via ``jwt.key_refresh_max_response_size``, where the default is 1 MB. + The key refresh interval is set via the ``jwt.key_refresh_interval`` configuration entry, where the default is 30 min (1800 seconds). Failed refreshes are retried after the lesser of 5 seconds and the configured interval, with exponential backoff capped at the configured interval. The maximum response body size accepted when fetching OpenID metadata and JWKS is set via ``jwt.key_refresh_max_response_size``, where the default is 1 MB. Removing a token issuer ----------------------- diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index f1b67abf575d..9eb70c682ee5 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -4,6 +4,7 @@ #include "ccf/ds/json.h" #include "ccf/ds/nonstd.h" +#include "ccf/pal/locking.h" #include "ccf/service/tables/jwt.h" #include "http/curl.h" #include "http/http_builder.h" @@ -13,8 +14,10 @@ #include "tasks/task_system.h" #define FMT_HEADER_ONLY +#include #include #include +#include namespace ccf { @@ -34,9 +37,130 @@ namespace ccf ccf::tasks::Task periodic_refresh_task; + struct RetryState + { + size_t delay_s; + size_t generation = 0; + ccf::tasks::Task task = nullptr; + }; + + ccf::pal::Mutex retry_states_lock; + std::map retry_states + CCF_GUARDED_BY(retry_states_lock); + size_t next_retry_generation CCF_GUARDED_BY(retry_states_lock) = 0; + + static constexpr size_t initial_retry_delay_s = 5; static constexpr long request_connection_timeout_s = 5; static constexpr long request_response_timeout_s = 5; + bool begin_retry(const JwtIssuer& issuer, size_t generation) + { + ccf::pal::MutexGuard guard(retry_states_lock); + const auto it = retry_states.find(issuer); + if ( + it == retry_states.end() || it->second.generation != generation || + it->second.task == nullptr) + { + return false; + } + + it->second.task = nullptr; + return true; + } + + void cancel_retry(const JwtIssuer& issuer) + { + ccf::pal::MutexGuard guard(retry_states_lock); + const auto it = retry_states.find(issuer); + if (it == retry_states.end()) + { + return; + } + + if (it->second.task != nullptr) + { + it->second.task->cancel_task(); + } + retry_states.erase(it); + } + + void cancel_retries() + { + ccf::pal::MutexGuard guard(retry_states_lock); + for (auto& retry_state_entry : retry_states) + { + auto& retry_state = retry_state_entry.second; + if (retry_state.task != nullptr) + { + retry_state.task->cancel_task(); + } + } + retry_states.clear(); + } + + void schedule_retry(const JwtIssuer& issuer) + { + ccf::tasks::Task retry_task; + size_t delay_s = 0; + { + ccf::pal::MutexGuard guard(retry_states_lock); + if (stopped.load()) + { + return; + } + + const auto initial_delay_s = + std::min(initial_retry_delay_s, refresh_interval_s); + const auto it = + retry_states + .try_emplace(issuer, RetryState{initial_delay_s, 0, nullptr}) + .first; + auto& retry_state = it->second; + if (retry_state.task != nullptr && !retry_state.task->is_cancelled()) + { + return; + } + + delay_s = retry_state.delay_s; + const auto generation = ++next_retry_generation; + retry_state.generation = generation; + const auto self = weak_from_this(); + retry_task = ccf::tasks::make_basic_task([self, issuer, generation]() { + const auto self_sp = self.lock(); + if ( + self_sp == nullptr || self_sp->stopped.load() || + !self_sp->begin_retry(issuer, generation)) + { + return; + } + + if (!self_sp->consensus->can_replicate()) + { + LOG_DEBUG_FMT( + "JWT key auto-refresh retry: Node is not primary, skipping"); + self_sp->cancel_retry(issuer); + return; + } + + self_sp->refresh_jwt_keys(issuer); + }); + retry_state.task = retry_task; + + if (retry_state.delay_s < refresh_interval_s) + { + retry_state.delay_s = retry_state.delay_s > refresh_interval_s / 2 ? + refresh_interval_s : + retry_state.delay_s * 2; + } + } + + LOG_DEBUG_FMT( + "JWT key auto-refresh: Scheduling retry for issuer '{}' in {}s", + issuer, + delay_s); + ccf::tasks::add_delayed_task(retry_task, std::chrono::seconds(delay_s)); + } + void send_curl_get( const std::string& url, const std::string& ca_bundle_pem, @@ -137,6 +261,7 @@ namespace ccf { periodic_refresh_task->cancel_task(); } + cancel_retries(); } void schedule_once() @@ -163,7 +288,7 @@ namespace ccf } template - void send_refresh_jwt_keys(T msg) + bool send_refresh_jwt_keys(T msg) { ::http::Request request(fmt::format( "/{}/{}", @@ -185,14 +310,17 @@ namespace ccf ::http::fetch_rpc_handler(ctx, this->rpc_map); search->process(ctx); + return ::http::status_success( + static_cast(ctx->get_response_status())); } - void send_refresh_jwt_keys_error() + void send_refresh_jwt_keys_error(const JwtIssuer& issuer) { // A message that the endpoint fails to parse, leading to 500. // This is done purely for exposing errors as endpoint metrics. auto msg = false; send_refresh_jwt_keys(msg); + schedule_retry(issuer); } void handle_jwt_jwks_response( @@ -210,7 +338,7 @@ namespace ccf data.empty() ? "" : fmt::format(" '{}'", std::string(data.begin(), data.end()))); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } @@ -228,7 +356,7 @@ namespace ccf "JWT key auto-refresh: Cannot parse JWKS for issuer '{}': {}", issuer, e.what()); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } @@ -248,7 +376,14 @@ namespace ccf // call internal endpoint to update keys auto msg = SetJwtPublicSigningKeys{issuer, jwks}; - send_refresh_jwt_keys(msg); + if (send_refresh_jwt_keys(msg)) + { + cancel_retry(issuer); + } + else + { + schedule_retry(issuer); + } } void handle_jwt_metadata_response( @@ -267,7 +402,7 @@ namespace ccf data.empty() ? "" : fmt::format(" '{}'", std::string(data.begin(), data.end()))); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } @@ -295,7 +430,7 @@ namespace ccf "{}", issuer, e.what()); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } // Validate jwks_uri before handing it to libcurl; the parsed result is @@ -315,7 +450,7 @@ namespace ccf issuer, jwks_url_str, e.what()); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } @@ -326,7 +461,7 @@ namespace ccf "JWT key auto-refresh: jwks_uri for issuer '{}' must use https: {}", issuer, jwks_url_str); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return; } @@ -365,7 +500,7 @@ namespace ccf issuer, curl_easy_strerror(curl_response), curl_response); - self_sp->send_refresh_jwt_keys_error(); + self_sp->send_refresh_jwt_keys_error(issuer); return; } self_sp->handle_jwt_jwks_response( @@ -379,7 +514,8 @@ namespace ccf send_curl_get(jwks_url_str, ca_bundle_pem, std::move(response_callback)); } - void refresh_jwt_keys() + void refresh_jwt_keys( + const std::optional& issuer_filter = std::nullopt) { if (stopped.load()) { @@ -389,7 +525,11 @@ namespace ccf auto tx = network.tables->create_read_only_tx(); auto* jwt_issuers = tx.ro(network.jwt_issuers); auto* ca_cert_bundles = tx.ro(network.ca_cert_bundles); - jwt_issuers->foreach([this, &ca_cert_bundles]( + bool issuer_found = !issuer_filter.has_value(); + jwt_issuers->foreach([this, + &ca_cert_bundles, + &issuer_filter, + &issuer_found]( const JwtIssuer& issuer, const JwtIssuerMetadata& metadata) { if (stopped.load()) @@ -397,12 +537,19 @@ namespace ccf return false; } + if (issuer_filter.has_value() && issuer != issuer_filter.value()) + { + return true; + } + issuer_found = true; + if (!metadata.auto_refresh) { LOG_DEBUG_FMT( "JWT key auto-refresh: Skipping issuer '{}', auto-refresh is " "disabled", issuer); + cancel_retry(issuer); return true; } @@ -421,7 +568,7 @@ namespace ccf "found", ca_cert_bundle_name, issuer); - send_refresh_jwt_keys_error(); + send_refresh_jwt_keys_error(issuer); return true; } @@ -465,7 +612,7 @@ namespace ccf issuer, curl_easy_strerror(curl_response), curl_response); - self_sp->send_refresh_jwt_keys_error(); + self_sp->send_refresh_jwt_keys_error(issuer); return; } self_sp->handle_jwt_metadata_response( @@ -480,6 +627,11 @@ namespace ccf metadata_url, ca_bundle_pem, std::move(response_callback)); return true; }); + + if (!issuer_found) + { + cancel_retry(issuer_filter.value()); + } } // Returns a copy of the current attempts diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 8921369461eb..3f8dc610f8b3 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -451,6 +451,7 @@ def test_jwt_key_auto_refresh_connection_failure(network, args): remove_all_jwt_issuers(network, args, primary) failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] issuer_host = "127.0.0.1" + kid = "connection_failure" LOG.info("Add JWT issuer with auto-refresh pointing at an unavailable endpoint") with reserve_unlistened_local_port() as issuer_port: @@ -458,13 +459,23 @@ def test_jwt_key_auto_refresh_connection_failure(network, args): f"https://{issuer_host}:{issuer_port}", cn=issuer_host ) add_auto_refresh_jwt_issuer(network, primary, issuer, "jwt_connection_failure") - try: + + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + + LOG.info("Start the OpenID endpoint and check that the refresh is retried") + with issuer.start_openid_server(issuer_port, kid): with_timeout( - lambda: check_refresh_failures_increased(primary, failures_before), - timeout=5, + lambda: check_kv_jwt_key_matches( + args, network, kid, issuer.key_pub_pem + ), + timeout=15, ) - finally: - network.consortium.remove_jwt_issuer(primary, issuer.name) + finally: + network.consortium.remove_jwt_issuer(primary, issuer.name) def test_jwt_key_auto_refresh_tls_failure(network, args): From 91a021daae1d9b65b6bc0b210b291d6b8066bc77 Mon Sep 17 00:00:00 2001 From: achamayou Date: Tue, 1 Sep 2026 14:46:58 +0100 Subject: [PATCH 3/6] Address review feedback on JWT refresh retries - Keep the reserved local port bound until the OpenID server is about to bind it, so nothing else can claim it while the initial refresh failures are observed. - Look up the retried issuer directly instead of scanning every issuer, removing the optional filter and the issuer_found bookkeeping. - Handle a missing ca_cert_bundle_name rather than unwrapping the optional, which a non-default constitution could leave unset. - Simplify the backoff doubling. - Move the changelog entry to a new 7.0.14 section and keep python/pyproject.toml in sync. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: f8de7f43-1588-4696-98fb-467b41ad04c2 --- CHANGELOG.md | 9 +- python/pyproject.toml | 2 +- src/node/jwt_key_auto_refresh.h | 229 +++++++++++++++++--------------- tests/jwt_test.py | 40 +++--- 4 files changed, 156 insertions(+), 124 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aba7ffbe291c..bc4695f900f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [7.0.14] + +[7.0.14]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.14 + +### Fixed + +- JWT/JWK auto-refresh failures are now retried after the lesser of 5 seconds and the configured key refresh interval, with exponential backoff capped at that interval (#8226, #3869). + ## [7.0.13] [7.0.13]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.13 @@ -27,7 +35,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Nodes from the previous service are now removed during disaster recovery instead of being retained as retired entries in `GET /node/network/nodes`, and `ledger_code.py` reports their code identities as removed (#8177). - Fixed an edge case where a follower could incorrectly commit to an abandoned fork while synchronising with the leader, causing it to become unavailable (#8172). -- JWT/JWK auto-refresh failures are now retried after the lesser of 5 seconds and the configured key refresh interval, with exponential backoff capped at that interval. (#8226, #3869) ## [7.0.12] diff --git a/python/pyproject.toml b/python/pyproject.toml index 18462482f034..7529d0383b9b 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "ccf" -version = "7.0.13" +version = "7.0.14" authors = [ { name="CCF Team", email="CCF-Sec@microsoft.com" }, ] diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 9eb70c682ee5..d2144115518c 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -5,6 +5,7 @@ #include "ccf/ds/json.h" #include "ccf/ds/nonstd.h" #include "ccf/pal/locking.h" +#include "ccf/service/tables/cert_bundles.h" #include "ccf/service/tables/jwt.h" #include "http/curl.h" #include "http/http_builder.h" @@ -148,9 +149,8 @@ namespace ccf if (retry_state.delay_s < refresh_interval_s) { - retry_state.delay_s = retry_state.delay_s > refresh_interval_s / 2 ? - refresh_interval_s : - retry_state.delay_s * 2; + retry_state.delay_s = + std::min(retry_state.delay_s * 2, refresh_interval_s); } } @@ -514,124 +514,143 @@ namespace ccf send_curl_get(jwks_url_str, ca_bundle_pem, std::move(response_callback)); } - void refresh_jwt_keys( - const std::optional& issuer_filter = std::nullopt) + void refresh_issuer_jwt_keys( + const JwtIssuer& issuer, + const JwtIssuerMetadata& metadata, + ccf::CACertBundlePEMs::ReadOnlyHandle* ca_cert_bundles) { - if (stopped.load()) + if (!metadata.auto_refresh) { + LOG_DEBUG_FMT( + "JWT key auto-refresh: Skipping issuer '{}', auto-refresh is " + "disabled", + issuer); + cancel_retry(issuer); return; } - auto tx = network.tables->create_read_only_tx(); - auto* jwt_issuers = tx.ro(network.jwt_issuers); - auto* ca_cert_bundles = tx.ro(network.ca_cert_bundles); - bool issuer_found = !issuer_filter.has_value(); - jwt_issuers->foreach([this, - &ca_cert_bundles, - &issuer_filter, - &issuer_found]( - const JwtIssuer& issuer, - const JwtIssuerMetadata& metadata) { - if (stopped.load()) - { - return false; - } + // Increment attempts, only when auto-refresh is enabled. + attempts++; - if (issuer_filter.has_value() && issuer != issuer_filter.value()) - { - return true; - } - issuer_found = true; + LOG_DEBUG_FMT( + "JWT key auto-refresh: Refreshing keys for issuer '{}'", issuer); + if (!metadata.ca_cert_bundle_name.has_value()) + { + LOG_INFO_FMT( + "JWT key auto-refresh: Issuer '{}' has auto-refresh enabled but no " + "CA cert bundle name", + issuer); + send_refresh_jwt_keys_error(issuer); + return; + } + const auto& ca_cert_bundle_name = metadata.ca_cert_bundle_name.value(); + auto ca_cert_bundle_pem = ca_cert_bundles->get(ca_cert_bundle_name); + if (!ca_cert_bundle_pem.has_value()) + { + LOG_INFO_FMT( + "JWT key auto-refresh: CA cert bundle with name '{}' for issuer " + "'{}' not " + "found", + ca_cert_bundle_name, + issuer); + send_refresh_jwt_keys_error(issuer); + return; + } - if (!metadata.auto_refresh) - { - LOG_DEBUG_FMT( - "JWT key auto-refresh: Skipping issuer '{}', auto-refresh is " - "disabled", - issuer); - cancel_retry(issuer); - return true; - } + auto metadata_url = issuer + "/.well-known/openid-configuration"; - // Increment attempts, only when auto-refresh is enabled. - attempts++; + LOG_DEBUG_FMT( + "JWT key auto-refresh: Requesting OpenID metadata at {}", metadata_url); - LOG_DEBUG_FMT( - "JWT key auto-refresh: Refreshing keys for issuer '{}'", issuer); - const auto& ca_cert_bundle_name = metadata.ca_cert_bundle_name.value(); - auto ca_cert_bundle_pem = ca_cert_bundles->get(ca_cert_bundle_name); - if (!ca_cert_bundle_pem.has_value()) - { - LOG_INFO_FMT( - "JWT key auto-refresh: CA cert bundle with name '{}' for issuer " - "'{}' not " - "found", - ca_cert_bundle_name, - issuer); - send_refresh_jwt_keys_error(issuer); - return true; - } + auto ca_bundle_pem = ca_cert_bundle_pem.value(); + + const auto self = weak_from_this(); + auto response_callback = [self, issuer, ca_bundle_pem]( + std::unique_ptr&& + request, + CURLcode curl_response, + long status_code) { + auto http_status = static_cast(status_code); + auto response_body_sp = std::make_shared>( + request->get_response_body() != nullptr ? + std::move(request->get_response_body()->buffer) : + std::vector{}); + ccf::tasks::add_task(ccf::tasks::make_basic_task([self, + issuer, + ca_bundle_pem, + curl_response, + http_status, + response_body_sp]() { + const auto self_sp = self.lock(); + if (self_sp == nullptr || self_sp->stopped.load()) + { + return; + } - auto metadata_url = issuer + "/.well-known/openid-configuration"; + if (curl_response != CURLE_OK) + { + LOG_INFO_FMT( + "JWT key auto-refresh: Failed to fetch OpenID metadata for " + "issuer '{}': {} ({})", + issuer, + curl_easy_strerror(curl_response), + curl_response); + self_sp->send_refresh_jwt_keys_error(issuer); + return; + } + self_sp->handle_jwt_metadata_response( + issuer, ca_bundle_pem, http_status, std::move(*response_body_sp)); + })); + }; - LOG_DEBUG_FMT( - "JWT key auto-refresh: Requesting OpenID metadata at {}", - metadata_url); + send_curl_get(metadata_url, ca_bundle_pem, std::move(response_callback)); + } - auto ca_bundle_pem = ca_cert_bundle_pem.value(); + void refresh_jwt_keys() + { + if (stopped.load()) + { + return; + } - const auto self = weak_from_this(); - auto response_callback = - [self, issuer, ca_bundle_pem]( - std::unique_ptr&& request, - CURLcode curl_response, - long status_code) { - auto http_status = static_cast(status_code); - auto response_body_sp = std::make_shared>( - request->get_response_body() != nullptr ? - std::move(request->get_response_body()->buffer) : - std::vector{}); - ccf::tasks::add_task( - ccf::tasks::make_basic_task([self, - issuer, - ca_bundle_pem, - curl_response, - http_status, - response_body_sp]() { - const auto self_sp = self.lock(); - if (self_sp == nullptr || self_sp->stopped.load()) - { - return; - } - - if (curl_response != CURLE_OK) - { - LOG_INFO_FMT( - "JWT key auto-refresh: Failed to fetch OpenID metadata for " - "issuer '{}': {} ({})", - issuer, - curl_easy_strerror(curl_response), - curl_response); - self_sp->send_refresh_jwt_keys_error(issuer); - return; - } - self_sp->handle_jwt_metadata_response( - issuer, - ca_bundle_pem, - http_status, - std::move(*response_body_sp)); - })); - }; - - send_curl_get( - metadata_url, ca_bundle_pem, std::move(response_callback)); - return true; - }); + auto tx = network.tables->create_read_only_tx(); + auto* jwt_issuers = tx.ro(network.jwt_issuers); + auto* ca_cert_bundles = tx.ro(network.ca_cert_bundles); + jwt_issuers->foreach( + [this, &ca_cert_bundles]( + const JwtIssuer& issuer, const JwtIssuerMetadata& metadata) { + if (stopped.load()) + { + return false; + } + + refresh_issuer_jwt_keys(issuer, metadata, ca_cert_bundles); + return true; + }); + } + + void refresh_jwt_keys(const JwtIssuer& issuer) + { + if (stopped.load()) + { + return; + } - if (!issuer_found) + auto tx = network.tables->create_read_only_tx(); + auto* jwt_issuers = tx.ro(network.jwt_issuers); + const auto metadata = jwt_issuers->get(issuer); + if (!metadata.has_value()) { - cancel_retry(issuer_filter.value()); + LOG_DEBUG_FMT( + "JWT key auto-refresh: Issuer '{}' is no longer registered, " + "abandoning retries", + issuer); + cancel_retry(issuer); + return; } + + refresh_issuer_jwt_keys( + issuer, metadata.value(), tx.ro(network.ca_cert_bundles)); } // Returns a copy of the current attempts diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 3f8dc610f8b3..9c9b3622057d 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -410,9 +410,12 @@ def get_jwt_refresh_endpoint_metrics(primary) -> dict: @contextmanager def reserve_unlistened_local_port(): + # The socket is bound but never listened on, so connections to it are + # refused. It is yielded rather than just its port number so that callers + # can release the reservation early, e.g. to let a server bind that port. with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.bind(("127.0.0.1", 0)) - yield s.getsockname()[1] + yield s def add_auto_refresh_jwt_issuer(network, primary, issuer, ca_cert_bundle_name): @@ -454,28 +457,31 @@ def test_jwt_key_auto_refresh_connection_failure(network, args): kid = "connection_failure" LOG.info("Add JWT issuer with auto-refresh pointing at an unavailable endpoint") - with reserve_unlistened_local_port() as issuer_port: + with reserve_unlistened_local_port() as reserved_socket: + issuer_port = reserved_socket.getsockname()[1] issuer = infra.jwt_issuer.JwtIssuer( f"https://{issuer_host}:{issuer_port}", cn=issuer_host ) add_auto_refresh_jwt_issuer(network, primary, issuer, "jwt_connection_failure") - - try: - with_timeout( - lambda: check_refresh_failures_increased(primary, failures_before), - timeout=5, - ) - - LOG.info("Start the OpenID endpoint and check that the refresh is retried") - with issuer.start_openid_server(issuer_port, kid): + try: with_timeout( - lambda: check_kv_jwt_key_matches( - args, network, kid, issuer.key_pub_pem - ), - timeout=15, + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, ) - finally: - network.consortium.remove_jwt_issuer(primary, issuer.name) + + LOG.info("Start the OpenID endpoint and check that the refresh is retried") + # Only release the port now, so that nothing else can claim it while + # the initial refresh failures are observed. + reserved_socket.close() + with issuer.start_openid_server(issuer_port, kid): + with_timeout( + lambda: check_kv_jwt_key_matches( + args, network, kid, issuer.key_pub_pem + ), + timeout=15, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer.name) def test_jwt_key_auto_refresh_tls_failure(network, args): From 93e2345aaf31c240385412b76d4fa58de8da91f3 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 8 Sep 2026 14:16:24 +0100 Subject: [PATCH 4/6] Clarify JWT retry scheduling and cover backoff decisions Rename the refresh interval cap and retry predicate, explain callback generations and shutdown locking, and add deterministic retry lifecycle coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CMakeLists.txt | 9 + src/node/jwt_key_auto_refresh.h | 29 ++- src/node/test/jwt_key_auto_refresh.cpp | 269 +++++++++++++++++++++++++ 3 files changed, 297 insertions(+), 10 deletions(-) create mode 100644 src/node/test/jwt_key_auto_refresh.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index dee27f1f816e..e4b372f4661e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -983,6 +983,15 @@ if(BUILD_TESTS) ) target_link_libraries(jwt_auth_test PRIVATE ccf_endpoints) + add_unit_test( + jwt_key_auto_refresh_test + ${CMAKE_CURRENT_SOURCE_DIR}/src/node/test/jwt_key_auto_refresh.cpp + ) + target_link_libraries( + jwt_key_auto_refresh_test + PRIVATE http_parser ccf_endpoints ccf_kv ccf_tasks curl uv + ) + add_unit_test( tx_status_test ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/tx_status_test.cpp diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index f188f6e775f4..7b67955fa053 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -7,6 +7,7 @@ #include "ccf/ds/nonstd.h" #include "ccf/service/tables/cert_bundles.h" #include "ccf/service/tables/jwt.h" +#include "enclave/rpc_map.h" #include "http/curl.h" #include "http/http_builder.h" #include "http/http_rpc_context.h" @@ -26,7 +27,7 @@ namespace ccf : public std::enable_shared_from_this { private: - size_t refresh_interval_s; + size_t max_refresh_interval_s; NetworkState& network; std::shared_ptr consensus; std::shared_ptr rpc_map; @@ -41,6 +42,9 @@ namespace ccf struct RetryState { size_t delay_s; + // Identifies the scheduled callback, not the issuer. Cancellation cannot + // stop a callback that has already started, so it must also check this + // ID. size_t generation = 0; ccf::tasks::Task task = nullptr; }; @@ -48,13 +52,14 @@ namespace ccf ccf::ds::Mutex retry_states_lock; std::map retry_states CCF_GUARDED_BY(retry_states_lock); + // Never reuse an ID, even after an issuer's retry state is erased. size_t next_retry_generation CCF_GUARDED_BY(retry_states_lock) = 0; static constexpr size_t initial_retry_delay_s = 5; static constexpr long request_connection_timeout_s = 5; static constexpr long request_response_timeout_s = 5; - bool begin_retry(const JwtIssuer& issuer, size_t generation) + bool should_begin_retry(const JwtIssuer& issuer, size_t generation) { ccf::ds::MutexGuard guard(retry_states_lock); const auto it = retry_states.find(issuer); @@ -65,6 +70,8 @@ namespace ccf return false; } + // Consume this callback's slot so a failed refresh can schedule the next + // retry, retaining the increased delay. it->second.task = nullptr; return true; } @@ -105,13 +112,15 @@ namespace ccf size_t delay_s = 0; { ccf::ds::MutexGuard guard(retry_states_lock); + // Keep this check under the lock: otherwise stop() could clear retries + // between the check and insertion, leaving a new retry after shutdown. if (stopped.load()) { return; } const auto initial_delay_s = - std::min(initial_retry_delay_s, refresh_interval_s); + std::min(initial_retry_delay_s, max_refresh_interval_s); const auto it = retry_states .try_emplace(issuer, RetryState{initial_delay_s, 0, nullptr}) @@ -130,7 +139,7 @@ namespace ccf const auto self_sp = self.lock(); if ( self_sp == nullptr || self_sp->stopped.load() || - !self_sp->begin_retry(issuer, generation)) + !self_sp->should_begin_retry(issuer, generation)) { return; } @@ -147,10 +156,10 @@ namespace ccf }); retry_state.task = retry_task; - if (retry_state.delay_s < refresh_interval_s) + if (retry_state.delay_s < max_refresh_interval_s) { retry_state.delay_s = - std::min(retry_state.delay_s * 2, refresh_interval_s); + std::min(retry_state.delay_s * 2, max_refresh_interval_s); } } @@ -201,14 +210,14 @@ namespace ccf public: JwtKeyAutoRefresh( - size_t refresh_interval_s, + size_t max_refresh_interval_s, NetworkState& network, const std::shared_ptr& consensus, const std::shared_ptr& rpc_map, ccf::crypto::ECKeyPairPtr node_sign_kp, ccf::crypto::Pem node_cert, size_t max_response_size) : - refresh_interval_s(refresh_interval_s), + max_refresh_interval_s(max_refresh_interval_s), network(network), consensus(consensus), rpc_map(rpc_map), @@ -247,10 +256,10 @@ namespace ccf LOG_DEBUG_FMT( "JWT key auto-refresh: Scheduling in {}s", - self_sp->refresh_interval_s); + self_sp->max_refresh_interval_s); }); - const std::chrono::seconds period(refresh_interval_s); + const std::chrono::seconds period(max_refresh_interval_s); ccf::tasks::add_periodic_task(periodic_refresh_task, period, period); } diff --git a/src/node/test/jwt_key_auto_refresh.cpp b/src/node/test/jwt_key_auto_refresh.cpp new file mode 100644 index 000000000000..289f8d94d76d --- /dev/null +++ b/src/node/test/jwt_key_auto_refresh.cpp @@ -0,0 +1,269 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. + +#include "node/jwt_key_auto_refresh.h" + +#include "kv/test/null_encryptor.h" +#include "kv/test/stub_consensus.h" + +#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN +#include + +using namespace std::chrono_literals; + +namespace +{ + class RefreshEndpoint : public ccf::RpcHandler + { + public: + bool accept_keys = true; + size_t key_updates = 0; + + void set_sig_intervals(size_t, size_t) override {} + void set_cmd_forwarder(std::shared_ptr) override {} + void open() override {} + bool is_open() override + { + return true; + } + void set_consensus_and_history( + ccf::kv::Consensus*, ccf::kv::TxHistory*) override + {} + + void process(std::shared_ptr ctx) override + { + const auto body = ccf::parse_json_safe(ctx->get_request_body()); + if (body.is_object()) + { + ++key_updates; + } + ctx->set_response_status( + body.is_object() && accept_keys ? HTTP_STATUS_OK : + HTTP_STATUS_INTERNAL_SERVER_ERROR); + } + }; + + struct Fixture + { + ccf::NetworkState network; + std::shared_ptr consensus = + std::make_shared(); + std::shared_ptr endpoint = + std::make_shared(); + std::shared_ptr refresh; + const ccf::JwtIssuer issuer = "https://issuer.example"; + + Fixture(size_t max_refresh_interval_s = 30) + { + network.tables->set_encryptor( + std::make_shared()); + consensus->force_become_primary(); + auto rpc_map = std::make_shared(); + rpc_map->register_frontend(endpoint); + refresh = std::make_shared( + max_refresh_interval_s, + network, + consensus, + rpc_map, + nullptr, + ccf::crypto::Pem{}, + 4096); + set_issuer(issuer); + } + + ~Fixture() + { + refresh.reset(); + advance(1h); + } + + void set_issuer(const ccf::JwtIssuer& name, bool auto_refresh = true) + { + auto tx = network.tables->create_tx(); + // A missing CA bundle makes each attempt fail synchronously, without + // wall-clock waits or external HTTP servers. + tx.rw(network.jwt_issuers) + ->put(name, ccf::JwtIssuerMetadata{std::nullopt, auto_refresh}); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + + void advance(std::chrono::milliseconds elapsed) + { + ccf::tasks::tick(elapsed); + auto& job_board = ccf::tasks::get_main_job_board(); + while (auto task = job_board.get_task()) + { + task->do_task(); + } + } + + void expect_retry_after(std::chrono::milliseconds delay) + { + const auto attempts = refresh->get_attempts(); + advance(delay - 1ms); + REQUIRE(refresh->get_attempts() == attempts); + advance(1ms); + REQUIRE(refresh->get_attempts() == attempts + 1); + } + + void respond_with_keys(const ccf::JwtIssuer& name) + { + const auto body = nlohmann::json(ccf::JsonWebKeySet{}).dump(); + refresh->handle_jwt_jwks_response( + name, + std::nullopt, + HTTP_STATUS_OK, + std::vector(body.begin(), body.end())); + } + + std::shared_ptr take_ready_retry() + { + auto task = std::dynamic_pointer_cast( + ccf::tasks::get_main_job_board().get_task()); + REQUIRE(task != nullptr); + return task; + } + }; +} + +TEST_CASE("JWT retries double their delay up to the configured maximum") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + REQUIRE(f.refresh->get_attempts() == 1); + for (const auto delay : {5s, 10s, 20s, 30s, 30s}) + { + f.expect_retry_after(delay); + } +} + +TEST_CASE("JWT retries respect a maximum below the initial retry delay") +{ + Fixture f(3); + f.refresh->refresh_jwt_keys(f.issuer); + f.expect_retry_after(3s); + f.expect_retry_after(3s); +} + +TEST_CASE("JWT refresh failures do not replace or advance a pending retry") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + f.advance(2s); + f.refresh->refresh_jwt_keys(f.issuer); + REQUIRE(f.refresh->get_attempts() == 2); + f.expect_retry_after(3s); + f.expect_retry_after(10s); +} + +TEST_CASE("JWT retry backoff and successful resets are independent per issuer") +{ + Fixture f; + const ccf::JwtIssuer other = "https://other.example"; + f.set_issuer(other); + f.refresh->refresh_jwt_keys(f.issuer); + f.expect_retry_after(5s); + + f.refresh->refresh_jwt_keys(other); + f.expect_retry_after(5s); + f.respond_with_keys(other); + REQUIRE(f.endpoint->key_updates == 1); + + f.expect_retry_after(5s); + f.refresh->refresh_jwt_keys(other); + f.expect_retry_after(5s); + f.respond_with_keys(other); + f.expect_retry_after(15s); +} + +TEST_CASE("JWT retries reset only after the keys are accepted") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + f.expect_retry_after(5s); + + f.endpoint->accept_keys = false; + f.respond_with_keys(f.issuer); + f.expect_retry_after(10s); + + f.endpoint->accept_keys = true; + f.respond_with_keys(f.issuer); + REQUIRE(f.endpoint->key_updates == 2); + const auto attempts = f.refresh->get_attempts(); + f.advance(30s); + REQUIRE(f.refresh->get_attempts() == attempts); + + f.refresh->refresh_jwt_keys(f.issuer); + f.expect_retry_after(5s); +} + +TEST_CASE("Stale JWT retry callbacks cannot consume a newer retry") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + ccf::tasks::tick(5s); + auto stale = f.take_ready_retry(); + + SUBCASE("A completed callback is invoked again") + { + stale->do_task(); + REQUIRE(f.refresh->get_attempts() == 2); + } + SUBCASE("Success cancels the callback and a later failure recreates state") + { + f.respond_with_keys(f.issuer); + REQUIRE(stale->is_cancelled()); + f.refresh->refresh_jwt_keys(f.issuer); + } + + const auto attempts = f.refresh->get_attempts(); + // Invoke the callback directly to model a worker that has already passed + // BaseTask's cancellation check before the newer retry was scheduled. + stale->fn(); + REQUIRE(f.refresh->get_attempts() == attempts); + f.expect_retry_after(stale->is_cancelled() ? 5s : 10s); +} + +TEST_CASE("JWT retries stop when the issuer or primary role is lost") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + + SUBCASE("Issuer removed") + { + auto tx = f.network.tables->create_tx(); + tx.rw(f.network.jwt_issuers)->remove(f.issuer); + REQUIRE(tx.commit() == ccf::kv::CommitResult::SUCCESS); + } + SUBCASE("Auto-refresh disabled") + { + f.set_issuer(f.issuer, false); + } + SUBCASE("Node is no longer primary") + { + f.consensus->state = ccf::kv::test::StubConsensus::Backup; + } + + f.advance(5s); + f.advance(30s); + REQUIRE(f.refresh->get_attempts() == 1); + + f.set_issuer(f.issuer); + f.consensus->force_become_primary(); + f.refresh->refresh_jwt_keys(f.issuer); + f.expect_retry_after(5s); +} + +TEST_CASE("Stopping JWT refresh cancels callbacks and prevents new retries") +{ + Fixture f; + f.refresh->refresh_jwt_keys(f.issuer); + ccf::tasks::tick(5s); + auto retry = f.take_ready_retry(); + f.refresh->stop(); + REQUIRE(retry->is_cancelled()); + retry->fn(); + f.refresh->send_refresh_jwt_keys_error(f.issuer); + f.advance(30s); + REQUIRE(f.refresh->get_attempts() == 1); +} From 10fcf58437fe4735879ac36707170a23e3545d10 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 8 Sep 2026 14:23:49 +0100 Subject: [PATCH 5/6] Keep JWT refresh interval naming consistent with configuration Restore refresh_interval_s in the implementation and test fixture while retaining the other review improvements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 16 ++++++++-------- src/node/test/jwt_key_auto_refresh.cpp | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 7b67955fa053..ee1e290e6c83 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -27,7 +27,7 @@ namespace ccf : public std::enable_shared_from_this { private: - size_t max_refresh_interval_s; + size_t refresh_interval_s; NetworkState& network; std::shared_ptr consensus; std::shared_ptr rpc_map; @@ -120,7 +120,7 @@ namespace ccf } const auto initial_delay_s = - std::min(initial_retry_delay_s, max_refresh_interval_s); + std::min(initial_retry_delay_s, refresh_interval_s); const auto it = retry_states .try_emplace(issuer, RetryState{initial_delay_s, 0, nullptr}) @@ -156,10 +156,10 @@ namespace ccf }); retry_state.task = retry_task; - if (retry_state.delay_s < max_refresh_interval_s) + if (retry_state.delay_s < refresh_interval_s) { retry_state.delay_s = - std::min(retry_state.delay_s * 2, max_refresh_interval_s); + std::min(retry_state.delay_s * 2, refresh_interval_s); } } @@ -210,14 +210,14 @@ namespace ccf public: JwtKeyAutoRefresh( - size_t max_refresh_interval_s, + size_t refresh_interval_s, NetworkState& network, const std::shared_ptr& consensus, const std::shared_ptr& rpc_map, ccf::crypto::ECKeyPairPtr node_sign_kp, ccf::crypto::Pem node_cert, size_t max_response_size) : - max_refresh_interval_s(max_refresh_interval_s), + refresh_interval_s(refresh_interval_s), network(network), consensus(consensus), rpc_map(rpc_map), @@ -256,10 +256,10 @@ namespace ccf LOG_DEBUG_FMT( "JWT key auto-refresh: Scheduling in {}s", - self_sp->max_refresh_interval_s); + self_sp->refresh_interval_s); }); - const std::chrono::seconds period(max_refresh_interval_s); + const std::chrono::seconds period(refresh_interval_s); ccf::tasks::add_periodic_task(periodic_refresh_task, period, period); } diff --git a/src/node/test/jwt_key_auto_refresh.cpp b/src/node/test/jwt_key_auto_refresh.cpp index 289f8d94d76d..5ac0c1163499 100644 --- a/src/node/test/jwt_key_auto_refresh.cpp +++ b/src/node/test/jwt_key_auto_refresh.cpp @@ -53,7 +53,7 @@ namespace std::shared_ptr refresh; const ccf::JwtIssuer issuer = "https://issuer.example"; - Fixture(size_t max_refresh_interval_s = 30) + Fixture(size_t refresh_interval_s = 30) { network.tables->set_encryptor( std::make_shared()); @@ -61,7 +61,7 @@ namespace auto rpc_map = std::make_shared(); rpc_map->register_frontend(endpoint); refresh = std::make_shared( - max_refresh_interval_s, + refresh_interval_s, network, consensus, rpc_map, From 87f6338520d6ef042a83aa0d54dad5caebc9a436 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Tue, 8 Sep 2026 14:50:02 +0100 Subject: [PATCH 6/6] Tighten deterministic JWT retry coverage Exercise one-off and periodic refresh scheduling, initial response failures and rejected key updates. Assert shutdown leaves no runnable retries, including retries for multiple issuers, without tightening wall-clock timeouts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/test/jwt_key_auto_refresh.cpp | 125 ++++++++++++++++++++----- 1 file changed, 104 insertions(+), 21 deletions(-) diff --git a/src/node/test/jwt_key_auto_refresh.cpp b/src/node/test/jwt_key_auto_refresh.cpp index 5ac0c1163499..319b9b52f3d0 100644 --- a/src/node/test/jwt_key_auto_refresh.cpp +++ b/src/node/test/jwt_key_auto_refresh.cpp @@ -97,9 +97,11 @@ namespace } } - void expect_retry_after(std::chrono::milliseconds delay) + void expect_attempt_after(std::chrono::milliseconds delay) { const auto attempts = refresh->get_attempts(); + CAPTURE(delay.count()); + CAPTURE(attempts); advance(delay - 1ms); REQUIRE(refresh->get_attempts() == attempts); advance(1ms); @@ -133,7 +135,7 @@ TEST_CASE("JWT retries double their delay up to the configured maximum") REQUIRE(f.refresh->get_attempts() == 1); for (const auto delay : {5s, 10s, 20s, 30s, 30s}) { - f.expect_retry_after(delay); + f.expect_attempt_after(delay); } } @@ -141,19 +143,89 @@ TEST_CASE("JWT retries respect a maximum below the initial retry delay") { Fixture f(3); f.refresh->refresh_jwt_keys(f.issuer); - f.expect_retry_after(3s); - f.expect_retry_after(3s); + f.expect_attempt_after(3s); + f.expect_attempt_after(3s); } TEST_CASE("JWT refresh failures do not replace or advance a pending retry") { Fixture f; - f.refresh->refresh_jwt_keys(f.issuer); + f.refresh->schedule_once(); + f.advance(0ms); + REQUIRE(f.refresh->get_attempts() == 1); f.advance(2s); - f.refresh->refresh_jwt_keys(f.issuer); + f.refresh->schedule_once(); + f.advance(0ms); REQUIRE(f.refresh->get_attempts() == 2); - f.expect_retry_after(3s); - f.expect_retry_after(10s); + f.expect_attempt_after(3s); + f.expect_attempt_after(10s); +} + +TEST_CASE("Periodic JWT refresh failures preserve the pending retry deadline") +{ + Fixture f(12); + f.refresh->start(); + f.refresh->schedule_once(); + f.advance(0ms); + REQUIRE(f.refresh->get_attempts() == 1); + f.expect_attempt_after(5s); + + // The periodic failure at 12s must not replace the retry due at 15s. + f.expect_attempt_after(7s); + f.expect_attempt_after(3s); + // The same holds at 24s, with the retry delay now capped at 12s. + f.expect_attempt_after(9s); + f.expect_attempt_after(3s); +} + +TEST_CASE("JWT response failures schedule an initial retry") +{ + Fixture f; + SUBCASE("Metadata HTTP error") + { + f.refresh->handle_jwt_metadata_response( + f.issuer, "", HTTP_STATUS_SERVICE_UNAVAILABLE, {}); + } + SUBCASE("Malformed metadata") + { + f.refresh->handle_jwt_metadata_response(f.issuer, "", HTTP_STATUS_OK, {}); + } + SUBCASE("JWKS HTTP error") + { + f.refresh->handle_jwt_jwks_response( + f.issuer, std::nullopt, HTTP_STATUS_SERVICE_UNAVAILABLE, {}); + } + SUBCASE("Malformed JWKS") + { + f.refresh->handle_jwt_jwks_response( + f.issuer, std::nullopt, HTTP_STATUS_OK, {}); + } + + REQUIRE(f.endpoint->key_updates == 0); + REQUIRE(f.refresh->get_attempts() == 0); + f.expect_attempt_after(5s); + f.expect_attempt_after(10s); +} + +TEST_CASE("A JWT key update schedules an initial retry only if rejected") +{ + Fixture f; + SUBCASE("Accepted") + { + f.respond_with_keys(f.issuer); + ccf::tasks::tick(30s); + REQUIRE(f.refresh->get_attempts() == 0); + REQUIRE(ccf::tasks::get_main_job_board().get_task() == nullptr); + } + SUBCASE("Rejected") + { + f.endpoint->accept_keys = false; + f.respond_with_keys(f.issuer); + REQUIRE(f.refresh->get_attempts() == 0); + f.expect_attempt_after(5s); + f.expect_attempt_after(10s); + } + REQUIRE(f.endpoint->key_updates == 1); } TEST_CASE("JWT retry backoff and successful resets are independent per issuer") @@ -162,29 +234,29 @@ TEST_CASE("JWT retry backoff and successful resets are independent per issuer") const ccf::JwtIssuer other = "https://other.example"; f.set_issuer(other); f.refresh->refresh_jwt_keys(f.issuer); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); f.refresh->refresh_jwt_keys(other); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); f.respond_with_keys(other); REQUIRE(f.endpoint->key_updates == 1); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); f.refresh->refresh_jwt_keys(other); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); f.respond_with_keys(other); - f.expect_retry_after(15s); + f.expect_attempt_after(15s); } TEST_CASE("JWT retries reset only after the keys are accepted") { Fixture f; f.refresh->refresh_jwt_keys(f.issuer); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); f.endpoint->accept_keys = false; f.respond_with_keys(f.issuer); - f.expect_retry_after(10s); + f.expect_attempt_after(10s); f.endpoint->accept_keys = true; f.respond_with_keys(f.issuer); @@ -194,7 +266,7 @@ TEST_CASE("JWT retries reset only after the keys are accepted") REQUIRE(f.refresh->get_attempts() == attempts); f.refresh->refresh_jwt_keys(f.issuer); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); } TEST_CASE("Stale JWT retry callbacks cannot consume a newer retry") @@ -221,7 +293,7 @@ TEST_CASE("Stale JWT retry callbacks cannot consume a newer retry") // BaseTask's cancellation check before the newer retry was scheduled. stale->fn(); REQUIRE(f.refresh->get_attempts() == attempts); - f.expect_retry_after(stale->is_cancelled() ? 5s : 10s); + f.expect_attempt_after(stale->is_cancelled() ? 5s : 10s); } TEST_CASE("JWT retries stop when the issuer or primary role is lost") @@ -251,19 +323,30 @@ TEST_CASE("JWT retries stop when the issuer or primary role is lost") f.set_issuer(f.issuer); f.consensus->force_become_primary(); f.refresh->refresh_jwt_keys(f.issuer); - f.expect_retry_after(5s); + f.expect_attempt_after(5s); } TEST_CASE("Stopping JWT refresh cancels callbacks and prevents new retries") { Fixture f; - f.refresh->refresh_jwt_keys(f.issuer); + const ccf::JwtIssuer other = "https://other.example"; + f.set_issuer(other); + f.refresh->start(); + f.refresh->refresh_jwt_keys(); + REQUIRE(f.refresh->get_attempts() == 2); ccf::tasks::tick(5s); auto retry = f.take_ready_retry(); + auto other_retry = f.take_ready_retry(); f.refresh->stop(); REQUIRE(retry->is_cancelled()); + REQUIRE(other_retry->is_cancelled()); retry->fn(); + other_retry->fn(); f.refresh->send_refresh_jwt_keys_error(f.issuer); - f.advance(30s); - REQUIRE(f.refresh->get_attempts() == 1); + f.refresh->send_refresh_jwt_keys_error(other); + ccf::tasks::tick(30s); + // Check the queue before running callbacks: their stopped checks could hide + // an incorrectly scheduled task from the attempts counter. + REQUIRE(ccf::tasks::get_main_job_board().get_task() == nullptr); + REQUIRE(f.refresh->get_attempts() == 2); }