Skip to content

Commit ed11711

Browse files
NilsBarlaugtilgovi
NilsBarlaug
and
tilgovi
committed
fix: use up-to-date kid in JWT header when refreshing
Co-authored-by: Randall Leeds <[email protected]>
1 parent 80f67e8 commit ed11711

File tree

22 files changed

+99
-118
lines changed

22 files changed

+99
-118
lines changed

‎cypress/helpers/index.js

+17
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,20 @@ const deleteGrant = (id) =>
9898
"DELETE",
9999
Cypress.env("admin_url") + "/trust/grants/jwt-bearer/issuers/" + id,
100100
)
101+
102+
export const validateJwt = (jwt) =>
103+
cy
104+
.request({
105+
method: "POST",
106+
url: `${Cypress.env("client_url")}/oauth2/validate-jwt`,
107+
form: true,
108+
body: { jwt },
109+
})
110+
.then(({ body }) => body)
111+
112+
export const rotateJwks = (set) =>
113+
cy
114+
.request("POST", `${Cypress.env("admin_url")}/keys/${set}`, {
115+
alg: "RS256",
116+
})
117+
.then(({ body }) => body)

‎cypress/integration/oauth2/jwt.js

+6-9
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright © 2022 Ory Corp
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import { createClient, prng } from "../../helpers"
4+
import { createClient, prng, validateJwt } from "../../helpers"
55

66
const accessTokenStrategies = ["opaque", "jwt"]
77

@@ -44,15 +44,12 @@ describe("OAuth 2.0 JSON Web Token Access Tokens", () => {
4444
expect(token.refresh_token).to.not.be.empty
4545
expect(token.access_token.split(".").length).to.equal(3)
4646
expect(token.refresh_token.split(".").length).to.equal(2)
47-
})
4847

49-
cy.request(`${Cypress.env("client_url")}/oauth2/validate-jwt`)
50-
.its("body")
51-
.then((body) => {
52-
console.log(body)
53-
expect(body.sub).to.eq("[email protected]")
54-
expect(body.client_id).to.eq(client.client_id)
55-
expect(body.jti).to.not.be.empty
48+
validateJwt(token.access_token).then(({ payload }) => {
49+
expect(payload.sub).to.eq("[email protected]")
50+
expect(payload.client_id).to.eq(client.client_id)
51+
expect(payload.jti).to.not.be.empty
52+
})
5653
})
5754
})
5855
})

‎cypress/integration/oauth2/refresh_token.js

+56-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright © 2022 Ory Corp
22
// SPDX-License-Identifier: Apache-2.0
33

4-
import { createClient, prng } from "../../helpers"
4+
import { createClient, prng, rotateJwks, validateJwt } from "../../helpers"
55

66
const accessTokenStrategies = ["opaque", "jwt"]
77

@@ -100,6 +100,61 @@ describe("The OAuth 2.0 Refresh Token Grant", function () {
100100
})
101101
})
102102
})
103+
104+
const validateJwtAndGetKid = (token) =>
105+
validateJwt(token).then(({ header }) => header.kid)
106+
107+
it("should refresh the Access and ID Token with newly rotated keys", function () {
108+
if (
109+
accessTokenStrategy === "opaque" ||
110+
(Cypress.env("jwt_enabled") !== "true" &&
111+
!Boolean(Cypress.env("jwt_enabled")))
112+
) {
113+
this.skip()
114+
}
115+
116+
const referrer = `${Cypress.env("client_url")}/empty`
117+
cy.visit(referrer, {
118+
failOnStatusCode: false,
119+
})
120+
121+
createClient({
122+
scope: "offline_access openid",
123+
redirect_uris: [referrer],
124+
grant_types: ["authorization_code", "refresh_token"],
125+
response_types: ["code"],
126+
token_endpoint_auth_method: "none",
127+
}).then((client) => {
128+
cy.authCodeFlowBrowser(client, {
129+
consent: {
130+
scope: ["offline_access", "openid"],
131+
},
132+
createClient: false,
133+
}).then(({ body: tokensBefore }) => {
134+
const kidsBefore = {
135+
accessToken: validateJwtAndGetKid(tokensBefore.access_token),
136+
idToken: validateJwtAndGetKid(tokensBefore.id_token),
137+
}
138+
139+
rotateJwks("hydra.jwt.access-token")
140+
rotateJwks("hydra.openid.id-token")
141+
142+
cy.refreshTokenBrowser(client, tokensBefore.refresh_token).then(
143+
({ body: tokensAfter }) => {
144+
const kidsAfter = {
145+
accessToken: validateJwtAndGetKid(tokensAfter.access_token),
146+
idToken: validateJwtAndGetKid(tokensAfter.id_token),
147+
}
148+
149+
expect(kidsAfter.accessToken).to.not.equal(
150+
kidsBefore.accessToken,
151+
)
152+
expect(kidsAfter.idToken).to.not.equal(kidsBefore.idToken)
153+
},
154+
)
155+
})
156+
})
157+
})
103158
})
104159
})
105160
})

‎cypress/support/commands.js

+2-3
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,9 @@ Cypress.Commands.add(
196196
})
197197
}
198198
if (doCreateClient) {
199-
createClient(client).then(run)
200-
return
199+
return createClient(client).then(run)
201200
}
202-
run(client)
201+
return run(client)
203202
},
204203
)
205204

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=0-description=should_pass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=0-description=should_pass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=2-description=should_pass_because_prompt=none_and_max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=2-description=should_pass_because_prompt=none_and_max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=5-description=should_pass_with_prompt=login_when_authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=jwt-case=5-description=should_pass_with_prompt=login_when_authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=0-description=should_pass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=0-description=should_pass_request_if_strategy_passes-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=2-description=should_pass_because_prompt=none_and_max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=2-description=should_pass_because_prompt=none_and_max_age_is_less_than_auth_time-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=5-description=should_pass_with_prompt=login_when_authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=legacy.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,7 @@
1818
"sid": ""
1919
}
2020
},
21-
"headers": {
22-
"extra": {
23-
}
24-
},
21+
"headers": null,
2522
"username": "",
2623
"subject": "foo"
2724
},

‎oauth2/.snapshots/TestAuthCodeWithMockStrategy-strategy=opaque-case=5-description=should_pass_with_prompt=login_when_authentication_time_is_recent-should_call_refresh_token_hook_if_configured-hook=new.json

+1-4
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717
"hooked": "legacy"
1818
}
1919
},
20-
"headers": {
21-
"extra": {
22-
}
23-
},
20+
"headers": null,
2421
"username": "",
2522
"subject": "foo"
2623
},

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

-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
"subject": "[email protected]"
3636
},
3737
"extra": {},
38-
"kid": "public:hydra.jwt.access-token",
3938
"client_id": "auth-code-client",
4039
"consent_challenge": "2261efbd447044a1b2f76b05c6aca164",
4140
"exclude_not_before_claim": false,

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

-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
"subject": "[email protected]"
3636
},
3737
"extra": {},
38-
"kid": "public:hydra.jwt.access-token",
3938
"client_id": "auth-code-client",
4039
"consent_challenge": "2261efbd447044a1b2f76b05c6aca164",
4140
"exclude_not_before_claim": false,

‎oauth2/handler.go

+1-47
Original file line numberDiff line numberDiff line change
@@ -701,15 +701,7 @@ func (h *Handler) getOidcUserInfo(w http.ResponseWriter, r *http.Request) {
701701
interim["jti"] = uuid.New()
702702
interim["iat"] = time.Now().Unix()
703703

704-
keyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
705-
if err != nil {
706-
h.r.Writer().WriteError(w, r, err)
707-
return
708-
}
709-
710-
token, _, err := h.r.OpenIDJWTStrategy().Generate(ctx, interim, &jwt.Headers{
711-
Extra: map[string]interface{}{"kid": keyID},
712-
})
704+
token, _, err := h.r.OpenIDJWTStrategy().Generate(ctx, interim, &jwt.Headers{})
713705
if err != nil {
714706
h.r.Writer().WriteError(w, r, err)
715707
return
@@ -1179,17 +1171,6 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
11791171
if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) ||
11801172
accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeJWTBearer)) ||
11811173
accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypePassword)) {
1182-
var accessTokenKeyID string
1183-
if h.c.AccessTokenStrategy(ctx, client.AccessTokenStrategySource(accessRequest.GetClient())) == "jwt" {
1184-
accessTokenKeyID, err = h.r.AccessTokenJWTStrategy().GetPublicKeyID(ctx)
1185-
if err != nil {
1186-
h.logOrAudit(err, r)
1187-
h.r.OAuth2Provider().WriteAccessError(ctx, w, accessRequest, err)
1188-
events.Trace(ctx, events.TokenExchangeError, events.WithRequest(accessRequest))
1189-
return
1190-
}
1191-
}
1192-
11931174
// only for client_credentials, otherwise Authentication is included in session
11941175
if accessRequest.GetGrantTypes().ExactOne(string(fosite.GrantTypeClientCredentials)) {
11951176
session.Subject = accessRequest.GetClient().GetID()
@@ -1207,7 +1188,6 @@ func (h *Handler) oauth2TokenExchange(w http.ResponseWriter, r *http.Request) {
12071188
}
12081189
}
12091190
session.ClientID = accessRequest.GetClient().GetID()
1210-
session.KID = accessTokenKeyID
12111191
session.DefaultSession.Claims.Issuer = h.c.IssuerURL(ctx).String()
12121192
session.DefaultSession.Claims.IssuedAt = time.Now().UTC()
12131193

@@ -1399,21 +1379,6 @@ func (h *Handler) updateSessionWithRequest(
13991379
request.GrantAudience(audience)
14001380
}
14011381

1402-
openIDKeyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
1403-
if err != nil {
1404-
x.LogError(r, err, h.r.Logger())
1405-
return nil, err
1406-
}
1407-
1408-
var accessTokenKeyID string
1409-
if h.c.AccessTokenStrategy(ctx, client.AccessTokenStrategySource(request.GetClient())) == "jwt" {
1410-
accessTokenKeyID, err = h.r.AccessTokenJWTStrategy().GetPublicKeyID(ctx)
1411-
if err != nil {
1412-
x.LogError(r, err, h.r.Logger())
1413-
return nil, err
1414-
}
1415-
}
1416-
14171382
obfuscatedSubject, err := h.r.ConsentStrategy().ObfuscateSubjectIdentifier(ctx, request.GetClient(), consent.ConsentRequest.Subject, consent.ConsentRequest.ForceSubjectIdentifier)
14181383
if e := &(fosite.RFC6749Error{}); errors.As(err, &e) {
14191384
x.LogAudit(r, err, h.r.AuditLogger())
@@ -1451,13 +1416,8 @@ func (h *Handler) updateSessionWithRequest(
14511416
session.DefaultSession = &openid.DefaultSession{}
14521417
}
14531418
session.DefaultSession.Claims = claims
1454-
session.DefaultSession.Headers = &jwt.Headers{Extra: map[string]interface{}{
1455-
// required for lookup on jwk endpoint
1456-
"kid": openIDKeyID,
1457-
}}
14581419
session.DefaultSession.Subject = consent.ConsentRequest.Subject
14591420
session.Extra = consent.Session.AccessToken
1460-
session.KID = accessTokenKeyID
14611421
session.ClientID = request.GetClient().GetID()
14621422
session.ConsentChallenge = consent.ConsentRequestID
14631423
session.ExcludeNotBeforeClaim = h.c.ExcludeNotBeforeClaim(ctx)
@@ -1618,13 +1578,7 @@ func (h *Handler) createVerifiableCredential(w http.ResponseWriter, r *http.Requ
16181578
}
16191579
}
16201580

1621-
signingKeyID, err := h.r.OpenIDJWTStrategy().GetPublicKeyID(ctx)
1622-
if err != nil {
1623-
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))
1624-
return
1625-
}
16261581
headers := jwt.NewHeaders()
1627-
headers.Add("kid", signingKeyID)
16281582
mapClaims, err := vcClaims.ToMapClaims()
16291583
if err != nil {
16301584
h.r.Writer().WriteError(w, r, errorsx.WithStack(err))

0 commit comments

Comments
 (0)