Skip to content

fix: refresh Google keys when kid rotates - #57

Closed
albertovincenzi wants to merge 1 commit into
fix/protect-logoutfrom
fix/refresh-unknown-jwks-kid
Closed

fix: refresh Google keys when kid rotates#57
albertovincenzi wants to merge 1 commit into
fix/protect-logoutfrom
fix/refresh-unknown-jwks-kid

Conversation

@albertovincenzi

Copy link
Copy Markdown
Collaborator

Problem

The JWKS cache returned any cached key set for a fixed hour. When Google rotated signing keys and issued a token with a new kid during that hour, Gate answered no matching key and rejected every affected login until the TTL elapsed or the process restarted.

The fetch path also accepted non-success HTTP responses and any JSON object as a cacheable key set, which could turn a transient upstream error into the same one-hour outage.

Fix

  • Use the fresh cache normally, but force one immediate JWKS download when the requested kid is absent or malformed.
  • Retry key selection exactly once and reject a genuinely unknown kid without looping.
  • Keep the existing stale-cache fallback for ordinary expiry refresh failures, where an old key may still validate a token.
  • Do not use stale data after an unknown-kid refresh fails, because that key set is already known not to contain the required key.
  • Reject non-2xx JWKS responses and JSON responses without a keys array instead of caching them.

Verification

Unit tests cover a rotated key appearing after refresh, a cache hit that performs no refresh, and an unknown key that stops after one retry.

  • cargo test --workspace
  • cargo test -p gate-server auth::tests
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo fmt --all -- --check

Review / dependency

This PR is stacked on #40 to avoid conflicts with the already-separated OAuth hardening series. Review commit 61f0b68 relative to fix/protect-logout. Merge order: #15, #31, #39, #40, then this PR.

@alice-viola

Copy link
Copy Markdown
Contributor

Landed on master via #67 (merge commit 944ee9b) as part of the 62-PR integration — this PR's head commit 61f0b68 is an ancestor of master. GitHub could not mark it merged automatically because its base is fix/protect-logout, not master. Closing as landed.

@alice-viola alice-viola closed this Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants