From 6b3194be54cc03d3328cf5f06e5d6ff1e6291941 Mon Sep 17 00:00:00 2001 From: NilsBarlaug Date: Wed, 12 Feb 2025 08:18:22 +0100 Subject: [PATCH] fix: use up-to-date `kid` in JWT header when refreshing Co-authored-by: Randall Leeds --- cypress/helpers/index.js | 13 ++++ cypress/integration/oauth2/jwt.js | 15 ++-- cypress/integration/oauth2/refresh_token.js | 75 ++++++++++++++++++- .../TestUnmarshalSession-v1.11.8.json | 1 - .../TestUnmarshalSession-v1.11.9.json | 1 - oauth2/handler.go | 49 +----------- oauth2/session.go | 5 +- oauth2/session_test.go | 1 - test/e2e/oauth2-client/src/index.js | 7 +- 9 files changed, 100 insertions(+), 67 deletions(-) diff --git a/cypress/helpers/index.js b/cypress/helpers/index.js index e8f11a9251f..4c03e783bd2 100644 --- a/cypress/helpers/index.js +++ b/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", + }) diff --git a/cypress/integration/oauth2/jwt.js b/cypress/integration/oauth2/jwt.js index 8b9d0fe78f8..5057d78f376 100644 --- a/cypress/integration/oauth2/jwt.js +++ b/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("foo@bar.com") - 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("foo@bar.com") + expect(payload.client_id).to.eq(client.client_id) + expect(payload.jti).to.not.be.empty + }) }) }) }) diff --git a/cypress/integration/oauth2/refresh_token.js b/cypress/integration/oauth2/refresh_token.js index 2ddf7d30f19..9fe35a4a3dc 100644 --- a/cypress/integration/oauth2/refresh_token.js +++ b/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) + }) + }) + }, + ) + }) + }) + }) }) }) }) diff --git a/oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json b/oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json index 03e8881ee72..0858db02c5f 100644 --- a/oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json +++ b/oauth2/.snapshots/TestUnmarshalSession-v1.11.8.json @@ -35,7 +35,6 @@ "subject": "foo@bar.com" }, "extra": {}, - "kid": "public:hydra.jwt.access-token", "client_id": "auth-code-client", "consent_challenge": "2261efbd447044a1b2f76b05c6aca164", "exclude_not_before_claim": false, diff --git a/oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json b/oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json index 03e8881ee72..0858db02c5f 100644 --- a/oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json +++ b/oauth2/.snapshots/TestUnmarshalSession-v1.11.9.json @@ -35,7 +35,6 @@ "subject": "foo@bar.com" }, "extra": {}, - "kid": "public:hydra.jwt.access-token", "client_id": "auth-code-client", "consent_challenge": "2261efbd447044a1b2f76b05c6aca164", "exclude_not_before_claim": false, diff --git a/oauth2/handler.go b/oauth2/handler.go index 0d4e4d37a3b..e8b1ee9c85c 100644 --- a/oauth2/handler.go +++ b/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)) diff --git a/oauth2/session.go b/oauth2/session.go index 0630cb09142..eb399b084f5 100644 --- a/oauth2/session.go +++ b/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 { diff --git a/oauth2/session_test.go b/oauth2/session_test.go index 461d753581a..9ad9acbc056 100644 --- a/oauth2/session_test.go +++ b/oauth2/session_test.go @@ -65,7 +65,6 @@ func TestUnmarshalSession(t *testing.T) { Subject: "foo@bar.com", }, Extra: map[string]interface{}{}, - KID: "public:hydra.jwt.access-token", ClientID: "auth-code-client", ConsentChallenge: "2261efbd447044a1b2f76b05c6aca164", ExcludeNotBeforeClaim: false, diff --git a/test/e2e/oauth2-client/src/index.js b/test/e2e/oauth2-client/src/index.js index d868fbc0d84..64dee2880e7 100644 --- a/test/e2e/oauth2-client/src/index.js +++ b/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 }