Skip to content

Retry failed JWT key refreshes with exponential backoff - #8226

Open
Amaury Chamayou (achamayou) with Copilot wants to merge 9 commits into
mainfrom
copilot/make-jwt-refresh-more-robust
Open

Retry failed JWT key refreshes with exponential backoff#8226
Amaury Chamayou (achamayou) with Copilot wants to merge 9 commits into
mainfrom
copilot/make-jwt-refresh-more-robust

Conversation

Copilot AI commented Aug 27, 2026 •
edited by achamayou
Loading

Copy link
Copy Markdown
Contributor

JWT key refresh previously waited for the configured periodic interval after failures, which defaults to 30 minutes. Existing outage coverage checked failure metrics but not subsequent recovery.

Changes

  • Retry scheduling

    • Track retries independently per issuer.
    • Retry after min(5 seconds, key_refresh_interval).
    • Exponentially back off to the configured interval.
    • Coalesce pending retries and discard stale tasks safely.
    • Cancel retry state after success, issuer removal, or role change.
    • Look up the retried issuer directly rather than scanning the whole issuer table.
    • Report a missing ca_cert_bundle_name as a refresh failure instead of unwrapping the optional, which a non-default constitution could leave unset.

  • Outage recovery coverage

    • Extend the unavailable-endpoint test to start the issuer after an initial connection failure.
    • Verify keys refresh before the deliberately long periodic interval expires.
    • Hold the reserved local port until the OpenID server is ready to bind it, so nothing else can claim it in between.

  • Documentation

    • Document retry behavior in the JWT operator guide and changelog (7.0.14).

Copilot AI linked an issue Aug 27, 2026 that may be closed by this pull request
Make JWT refresh more robust #3869
Open
@achamayou
Retry failed JWT key refreshes
1fd55cc 
Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com>
Copilot AI changed the title [WIP] Add retry logic for JWT refresh handling errors Retry failed JWT key refreshes with exponential backoff Aug 27, 2026
@achamayou
Amaury Chamayou (achamayou) marked this pull request as ready for review August 28, 2026 17:17
@achamayou
Amaury Chamayou (achamayou) requested a review from a team as a code owner August 28, 2026 17:17
Copilot AI lite review requested due to automatic review settings August 28, 2026 17:17
Copilot AI reviewed Aug 28, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR improves JWT/JWK auto-refresh robustness in CCF by adding per-issuer retry scheduling with exponential backoff, so transient OpenID/JWKS endpoint failures recover quickly rather than waiting for the (often long) periodic refresh interval.

Changes:

  • Add per-issuer retry state and delayed-task scheduling to retry failed refreshes quickly, then exponentially back off up to the configured refresh interval.
  • Extend the connection-failure e2e test to verify recovery once the OpenID endpoint becomes available.
  • Document the retry behavior in the JWT operator guide and add a changelog entry.

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md
  • .github/instructions/changelog.instructions.md

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/node/jwt_key_auto_refresh.h Adds per-issuer retry scheduling with exponential backoff and cancellation on success/issuer removal/role change.
tests/jwt_test.py Extends the connection-failure test to start the issuer after an initial failure and assert refresh recovery.
doc/build_apps/auth/jwt.rst Documents the new retry + exponential backoff behavior.
CHANGELOG.md Records the user-visible behavior change for JWT auto-refresh retries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/jwt_test.py Outdated
@achamayou
Address review feedback on JWT refresh retries
91a021d 
- 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
@achamayou Amaury Chamayou (achamayou) mentioned this pull request Sep 2, 2026
Closed
@achamayou
Merge latest main into JWT refresh retries
05b18bc 
Resolve the 7.0.14 changelog conflict and update retry locking to the ccf::ds API now used on main.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
maxtropets
Max (maxtropets) reviewed Sep 8, 2026
View reviewed changes
Comment thread src/node/jwt_key_auto_refresh.h
maxtropets
Max (maxtropets) reviewed Sep 8, 2026
View reviewed changes
Comment thread src/node/jwt_key_auto_refresh.h Outdated
maxtropets
Max (maxtropets) reviewed Sep 8, 2026
View reviewed changes
Comment thread src/node/jwt_key_auto_refresh.h Outdated
maxtropets
Max (maxtropets) reviewed Sep 8, 2026
View reviewed changes
Comment thread tests/jwt_test.py
maxtropets
Max (maxtropets) reviewed Sep 8, 2026
View reviewed changes
Comment thread src/node/jwt_key_auto_refresh.h
@achamayou
Clarify JWT retry scheduling and cover backoff decisions
93e2345 
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>
@achamayou
Keep JWT refresh interval naming consistent with configuration
10fcf58 
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>
@achamayou
Tighten deterministic JWT retry coverage
87f6338 
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>
@achamayou
Merge main and resolve http_client rename conflicts
1fb5936 
Main moved the curl client to src/http_client (#8285) and the HTTP RPC
context into src/enclave (#8284). Apply the ccf::curl -> ccf::http_client
rename and include moves to the refresh path extracted by this PR, and
move the changelog entry from the released 7.0.14 section to 7.0.16.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
maxtropets
Max (maxtropets) reviewed Sep 14, 2026
View reviewed changes
Comment thread tests/jwt_test.py
args, network, kid, issuer.key_pub_pem
),
timeout=15,
)

@maxtropets Max (maxtropets) Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have approved, but I'll leave this discussion open to make sure there's better test coverage, although I'm not insisting, feel free to close and merge as is.

@achamayou Amaury Chamayou (achamayou) Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The specifics of the timing are tested in detail here https://github.com/microsoft/CCF/pull/8226/changes#diff-af7d56e22a211a19785540ffa741b8699f0c4430f553189776c65f0d34e4b310R131, is there something specific that you think is not tested here and ought to be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@maxtropets Max (maxtropets) maxtropets approved these changes
@achamayou Amaury Chamayou (achamayou) Awaiting requested review from achamayou

Assignees

@achamayou Amaury Chamayou (achamayou)

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make JWT refresh more robust

4 participants

@achamayou @maxtropets