Skip to content

fix(auth): jwt kid privatekey mismatch (#104) - #154

Open
BKJN1 wants to merge 1 commit into
mainfrom
fix/104-jwt-kid-privatekey-mismatch
Open

fix(auth): jwt kid privatekey mismatch (#104)#154
BKJN1 wants to merge 1 commit into
mainfrom
fix/104-jwt-kid-privatekey-mismatch

Conversation

@BKJN1

@BKJN1 BKJN1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #104 — auth was labeling JWTs with a kid fetched dynamically from JwksService while signing them with a static config.jwt.privateKey. When central rotates the active key, auth picks up the new kid before it's redeployed with the matching private key, producing tokens whose kid points to a public key that never actually signed them ("ghost key") — verifiers would reject a token that looks correctly keyed but has an invalid signature.

Root cause

Same pattern was present in 5 places, not just the 2 listed in the issue:

  • TokenEndpointController.scala (access token + id token)
  • UserInfoController.scala (JWT userinfo response)
  • LogoutService.scala (back-channel logout token)
  • ConversationRenderService.scala (id_token on conversation complete)
  • AuthorizeEndpointService.scala (id_token, hybrid flow)

All five did:

signingKey <- jwksService.getPublicKeys.map(_.active)  // dynamic kid
...
signature = JWT.Signature.Asymmetric(
  keyId = signingKey.id,              // <- from JwksService
  privateKey = config.jwt.privateKey, // <- static config
)

Fix

Added keyId: String to CoreConfig.JwtConfig, so keyId and privateKey are always the matching pair auth signs with — mirrors the existing EdgeConfig.keyId/privateKey pattern already used for edge's own signing. All 5 sites now sign with config.jwt.keyId / JWT.Algorithm.RS256 directly, never with whatever JwksService currently reports as "active".

JwksService is untouched for verification (deserializing inbound tokens still uses the full dynamic key set — that's correct and unrelated to this bug). It's dropped entirely from LogoutService and ConversationRenderService, which no longer need it for anything.

This doesn't give zero-downtime key rotation (still requires a config/deploy step to stay in sync with central's JWKS) — but it makes drift fail safely: a mismatched keyId now produces an "unknown kid" rejection at verification time instead of a token that looks valid but isn't.

Other changes

  • scripts/gen-env.scala — emits jwt.key-id alongside jwt.private-key in the generated auth conf.

Tests

  • TestEnvConfig.scalajwtConfig now carries keyId.
  • LogoutServiceSpec.scala, ConversationRenderServiceSpec.scala — updated constructor calls for the new (smaller) arity.
  • TokenEndpointControllerSpec.scala — new regression test: JwksService reports an "active" key with a different, unrelated kid; asserts /token still signs with config.jwt.keyId and the signature verifies against the real key. Fails on the old code (kid would be the unrelated one), passes now.

@augmentcode

augmentcode Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor
🤖 Augment PR Summary

Summary: This PR fixes auth JWTs whose header kid could drift from the static private key used to sign them.

Changes:

  • Adds optional jwt.key-id configuration alongside auth’s existing JWT private key.
  • Uses that configured key ID and explicit RS256 for access, ID, userinfo, and back-channel logout JWTs.
  • Updates OIDC c_hash and at_hash generation to match RS256 signing.
  • Removes dynamic JWKS signing-key lookups from token generation paths that only need the configured key pair.
  • Keeps dynamic JWKS retrieval in verification paths, including incoming access-token and ID-token-hint validation.
  • Simplifies the conversation-render and logout service layers after removing their JWKS dependency.
  • Updates generated auth configuration to emit jwt.key-id.
  • Updates test configuration and constructor wiring for the new configuration/service signatures.
  • Adds a token-endpoint regression test covering JWKS active-key drift and missing key-ID behavior.
Technical Notes: Existing configurations without jwt.key-id continue to boot, but signing requests fail until the matching key ID is configured; this avoids producing misleading tokens with invalid signatures.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot 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.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

// together (not looked up dynamically from JwksService) so signing can never
// label a token with a kid that doesn't correspond to the private key that
// actually signed it. See EdgeConfig.keyId/privateKey for the same pattern.
keyId: String,

@augmentcode augmentcode Bot Aug 8, 2026

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.

keyId is now mandatory when parseConfig[CoreConfig] starts auth, but every existing auth config generated before this PR contains only jwt.issuer and jwt.private-key; deploying this image before the separately maintained runtime config is migrated will make auth fail to start.

Severity: high

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@BKJN1

BKJN1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

augment review

@augmentcode augmentcode Bot 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.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@BKJN1
BKJN1 force-pushed the fix/104-jwt-kid-privatekey-mismatch branch from 841dc8a to a80b14f Compare August 8, 2026 10:14
@BKJN1

BKJN1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

augment review

@augmentcode augmentcode Bot 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.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@BKJN1

BKJN1 commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator Author

@goshacodes
ready for approve

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.

auth: Mismatch between kid and private key during JWT signing due to dynamic JWKS reload

1 participant