fix: use up-to-date kid in JWT header when refreshing

Co-authored-by: Randall Leeds <[email protected]>

Author: NilsBarlaug

cypress/helpers/index.js

@@ -98,3 +98,16 @@ const deleteGrant = (id) =>
     "DELETE",
     Cypress.env("admin_url") + "/trust/grants/jwt-bearer/issuers/" + id,
   )
+
+export const validateJwt = (jwt) =>
+  cy.request({
+    method: "POST",
+    url: `${Cypress.env("client_url")}/oauth2/validate-jwt`,
+    form: true,
+    body: { jwt },
+  })
+
+export const rotateJwks = (set) =>
+  cy.request("POST", `${Cypress.env("admin_url")}/keys/${set}`, {
+    alg: "RS256",
+  })

cypress/integration/oauth2/jwt.js

@@ -1,7 +1,7 @@
 // Copyright © 2022 Ory Corp
 // SPDX-License-Identifier: Apache-2.0
 
-import { createClient, prng } from "../../helpers"
+import { createClient, prng, validateJwt } from "../../helpers"
 
 const accessTokenStrategies = ["opaque", "jwt"]
 
@@ -44,15 +44,12 @@ describe("OAuth 2.0 JSON Web Token Access Tokens", () => {
               expect(token.refresh_token).to.not.be.empty
               expect(token.access_token.split(".").length).to.equal(3)
               expect(token.refresh_token.split(".").length).to.equal(2)
-            })
 
-          cy.request(`${Cypress.env("client_url")}/oauth2/validate-jwt`)
-            .its("body")
-            .then((body) => {
-              console.log(body)
-              expect(body.sub).to.eq("[email protected]")
-              expect(body.client_id).to.eq(client.client_id)
-              expect(body.jti).to.not.be.empty
+              validateJwt(token.access_token).then(({ payload }) => {
+                expect(payload.sub).to.eq("[email protected]")
+                expect(payload.client_id).to.eq(client.client_id)
+                expect(payload.jti).to.not.be.empty
+              })
             })
         })
       })

cypress/integration/oauth2/refresh_token.js

@@ -1,7 +1,9 @@
 // Copyright © 2022 Ory Corp
 // SPDX-License-Identifier: Apache-2.0
 
-import { createClient, prng } from "../../helpers"
+import { validate as uuidValidate } from "uuid"
+
+import { createClient, prng, rotateJwks, validateJwt } from "../../helpers"
 
 const accessTokenStrategies = ["opaque", "jwt"]
 
@@ -100,6 +102,77 @@ describe("The OAuth 2.0 Refresh Token Grant", function () {
           })
         })
       })
+
+      it("should refresh the Access and ID Token with newly rotated keys", function () {
+        if (
+          accessTokenStrategy === "opaque" ||
+          (Cypress.env("jwt_enabled") !== "true" &&
+            !Boolean(Cypress.env("jwt_enabled")))
+        ) {
+          this.skip()
+        }
+
+        const referrer = `${Cypress.env("client_url")}/empty`
+        cy.visit(referrer, {
+          failOnStatusCode: false,
+        })
+
+        createClient({
+          scope: "offline_access openid",
+          redirect_uris: [referrer],
+          grant_types: ["authorization_code", "refresh_token"],
+          response_types: ["code"],
+          token_endpoint_auth_method: "none",
+        }).then((client) => {
+          cy.authCodeFlowBrowser(client, {
+            consent: {
+              scope: ["offline_access", "openid"],
+            },
+            createClient: false,
+          }).then((originalResponse) => {
+            expect(originalResponse.status).to.eq(200)
+            expect(originalResponse.body.refresh_token).to.not.be.empty
+
+            const originalToken = originalResponse.body.refresh_token
+
+            rotateJwks("hydra.jwt.access-token")
+            rotateJwks("hydra.openid.id-token")
+
+            cy.refreshTokenBrowser(client, originalToken).then(
+              (refreshedResponse) => {
+                expect(refreshedResponse.status).to.eq(200)
+                expect(refreshedResponse.body.refresh_token).to.not.be.empty
+
+                validateJwt(originalResponse.body.access_token)
+                  .its("body.header.kid")
+                  .then((originalKid) => {
+                    expect(originalKid).to.satisfy(uuidValidate)
+
+                    validateJwt(refreshedResponse.body.access_token)
+                      .its("body.header.kid")
+                      .then((refreshedKid) => {
+                        expect(refreshedKid).to.satisfy(uuidValidate)
+                        expect(refreshedKid).to.not.eq(originalKid)
+                      })
+                  })
+
+                validateJwt(originalResponse.body.id_token)
+                  .its("body.header.kid")
+                  .then((originalKid) => {
+                    expect(originalKid).to.satisfy(uuidValidate)
+
+                    validateJwt(refreshedResponse.body.id_token)
+                      .its("body.header.kid")
+                      .then((refreshedKid) => {
+                        expect(refreshedKid).to.satisfy(uuidValidate)
+                        expect(refreshedKid).to.not.eq(originalKid)
+                      })
+                  })
+              },
+            )
+          })
+        })
+      })
     })
   })
 })

oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json

@@ -35,7 +35,6 @@
     "subject": "[email protected]"
   },
   "extra": {},
-  "kid": "public:hydra.jwt.access-token",
   "client_id": "auth-code-client",
   "consent_challenge": "2261efbd447044a1b2f76b05c6aca164",
   "exclude_not_before_claim": false,

oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json

@@ -35,7 +35,6 @@
     "subject": "[email protected]"
   },
   "extra": {},
-  "kid": "public:hydra.jwt.access-token",
   "client_id": "auth-code-client",
   "consent_challenge": "2261efbd447044a1b2f76b05c6aca164",
   "exclude_not_before_claim": false,

oauth2/handler.go

@@ -701,15 +701,7 @@ func (h *Handler) getOidcUserInfo(w http.ResponseWriter, r *http.Request) {
 		interim["jti"] = uuid.New()
 		interim["iat"] = time.Now().Unix()
 
-		keyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
-		if err != nil {
-			h.r.Writer().WriteError(w, r, err)
-			return
-		}
-
-		token, _, err := h.r.OpenIDJWTStrategy().Generate(ctx, interim, &jwt.Headers{
-			Extra: map[string]interface{}{"kid": keyID},
-		})
+		token, _, err := h.r.OpenIDJWTStrategy().Generate(ctx, interim, &jwt.Headers{})
 		if err != nil {
 			h.r.Writer().WriteError(w, r, err)
 			return
@@ -1185,17 +1177,6 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
 	if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) ||
 		accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeJWTBearer)) ||
 		accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) {
-		var accessTokenKeyID string
-		if h.c.AccessTokenStrategy(ctx, client.AccessTokenStrategySource(accessRequest.GetClient())) == "jwt" {
-			accessTokenKeyID, err = h.r.AccessTokenJWTStrategy().GetPublicKeyID(ctx)
-			if err != nil {
-				h.logOrAudit(err, r)
-				h.r.OAuth2Provider().WriteAccessError(ctx, w, accessRequest, err)
-				events.Trace(ctx, events.TokenExchangeError, events.WithRequest(accessRequest), events.WithError(err))
-				return
-			}
-		}
-
 		// only for client_credentials, otherwise Authentication is included in session
 		if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) {
 			session.Subject = accessRequest.GetClient().GetID()
@@ -1213,7 +1194,6 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
 			}
 		}
 		session.ClientID = accessRequest.GetClient().GetID()
-		session.KID = accessTokenKeyID
 		session.DefaultSession.Claims.Issuer = h.c.IssuerURL(ctx).String()
 		session.DefaultSession.Claims.IssuedAt = time.Now().UTC()
 
@@ -1404,21 +1384,6 @@ func (h *Handler) updateSessionWithRequest(
 		request.GrantAudience(audience)
 	}
 
-	openIDKeyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
-	if err != nil {
-		x.LogError(r, err, h.r.Logger())
-		return nil, err
-	}
-
-	var accessTokenKeyID string
-	if h.c.AccessTokenStrategy(ctx, client.AccessTokenStrategySource(request.GetClient())) == "jwt" {
-		accessTokenKeyID, err = h.r.AccessTokenJWTStrategy().GetPublicKeyID(ctx)
-		if err != nil {
-			x.LogError(r, err, h.r.Logger())
-			return nil, err
-		}
-	}
-
 	obfuscatedSubject, err := h.r.ConsentStrategy().ObfuscateSubjectIdentifier(ctx, request.GetClient(), consent.ConsentRequest.Subject, consent.ConsentRequest.ForceSubjectIdentifier)
 	if e := &(fosite.RFC6749Error{}); errors.As(err, &e) {
 		x.LogAudit(r, err, h.r.AuditLogger())
@@ -1456,13 +1421,9 @@ func (h *Handler) updateSessionWithRequest(
 		session.DefaultSession = &openid.DefaultSession{}
 	}
 	session.DefaultSession.Claims = claims
-	session.DefaultSession.Headers = &jwt.Headers{Extra: map[string]interface{}{
-		// required for lookup on jwk endpoint
-		"kid": openIDKeyID,
-	}}
+	session.DefaultSession.Headers = jwt.NewHeaders()
 	session.DefaultSession.Subject = consent.ConsentRequest.Subject
 	session.Extra = consent.Session.AccessToken
-	session.KID = accessTokenKeyID
 	session.ClientID = request.GetClient().GetID()
 	session.ConsentChallenge = consent.ConsentRequestID
 	session.ExcludeNotBeforeClaim = h.c.ExcludeNotBeforeClaim(ctx)
@@ -1623,13 +1584,7 @@ func (h *Handler) createVerifiableCredential(w http.ResponseWriter, r *http.Requ
 		}
 	}
 
-	signingKeyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
-	if err != nil {
-		h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
-		return
-	}
 	headers := jwt.NewHeaders()
-	headers.Add("kid", signingKeyID)
 	mapClaims, err := vcClaims.ToMapClaims()
 	if err != nil {
 		h.r.Writer().WriteError(w, r, errorsx.WithStack(err))

oauth2/session.go

@@ -27,7 +27,6 @@ import (
 type Session struct {
 	*openid.DefaultSession `json:"id_token"`
 	Extra                  map[string]interface{} `json:"extra"`
-	KID                    string                 `json:"kid"`
 	ClientID               string                 `json:"client_id"`
 	ConsentChallenge       string                 `json:"consent_challenge"`
 	ExcludeNotBeforeClaim  bool                   `json:"exclude_not_before_claim"`
@@ -116,9 +115,7 @@ func (s *Session) GetJWTClaims() jwt.JWTClaimsContainer {
 }
 
 func (s *Session) GetJWTHeader() *jwt.Headers {
-	return &jwt.Headers{
-		Extra: map[string]interface{}{"kid": s.KID},
-	}
+	return jwt.NewHeaders()
 }
 
 func (s *Session) Clone() fosite.Session {

oauth2/session_test.go

@@ -65,7 +65,6 @@ func TestUnmarshalSession(t *testing.T) {
 			Subject:  "[email protected]",
 		},
 		Extra:                 map[string]interface{}{},
-		KID:                   "public:hydra.jwt.access-token",
 		ClientID:              "auth-code-client",
 		ConsentChallenge:      "2261efbd447044a1b2f76b05c6aca164",
 		ExcludeNotBeforeClaim: false,

test/e2e/oauth2-client/src/index.js

@@ -260,23 +260,24 @@ app.get("/oauth2/revoke", (req, res) => {
     })
 })
 
-app.get("/oauth2/validate-jwt", (req, res) => {
+app.post("/oauth2/validate-jwt", (req, res) => {
   const client = jwksClient({
     jwksUri: new URL("/.well-known/jwks.json", config.public).toString(),
   })
 
   jwt.verify(
-    req.session.oauth2_flow.token.access_token,
+    req.body.jwt,
     (header, callback) => {
       client.getSigningKey(header.kid, function (err, key) {
         const signingKey = key.publicKey || key.rsaPublicKey
         callback(null, signingKey)
       })
     },
+    { complete: true },
     (err, decoded) => {
       if (err) {
         console.error(err)
-        res.send(400)
+        res.status(400).send(JSON.stringify({ error: err.toString() }))
         return
       }