Skip to content

Commit fd6a68f

Browse files
arpit-jnarpit-jn
authored
fix: read and migrate v3 session format to v4 (#1923)
2 parents 33c210d + 515d714 commit fd6a68f

11 files changed

+673
-86
lines changed

‎src/server/auth-client.test.ts

Lines changed: 65 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
440440
// assert session has been updated
441441
const updatedSessionCookie = response.cookies.get("__session");
442442
expect(updatedSessionCookie).toBeDefined();
443-
const updatedSessionCookieValue = await decrypt(
443+
const { payload: updatedSessionCookieValue } = await decrypt(
444444
updatedSessionCookie!.value,
445445
secret
446446
);
@@ -795,13 +795,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
795795
`__txn_${authorizationUrl.searchParams.get("state")}`
796796
);
797797
expect(transactionCookie).toBeDefined();
798-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
799-
nonce: authorizationUrl.searchParams.get("nonce"),
800-
codeVerifier: expect.any(String),
801-
responseType: "code",
802-
state: authorizationUrl.searchParams.get("state"),
803-
returnTo: "/"
804-
});
798+
expect((await decrypt(transactionCookie!.value, secret)).payload).toEqual(
799+
{
800+
nonce: authorizationUrl.searchParams.get("nonce"),
801+
codeVerifier: expect.any(String),
802+
responseType: "code",
803+
state: authorizationUrl.searchParams.get("state"),
804+
returnTo: "/"
805+
}
806+
);
805807
});
806808

807809
it("should return an error if the discovery endpoint could not be fetched", async () => {
@@ -911,7 +913,9 @@ ca/T0LLtgmbMmxSv/MmzIg==
911913
`__txn_${authorizationUrl.searchParams.get("state")}`
912914
);
913915
expect(transactionCookie).toBeDefined();
914-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
916+
expect(
917+
(await decrypt(transactionCookie!.value, secret)).payload
918+
).toEqual({
915919
nonce: authorizationUrl.searchParams.get("nonce"),
916920
codeVerifier: expect.any(String),
917921
responseType: "code",
@@ -1243,14 +1247,16 @@ ca/T0LLtgmbMmxSv/MmzIg==
12431247
`__txn_${authorizationUrl.searchParams.get("state")}`
12441248
);
12451249
expect(transactionCookie).toBeDefined();
1246-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1247-
nonce: authorizationUrl.searchParams.get("nonce"),
1248-
maxAge: 3600,
1249-
codeVerifier: expect.any(String),
1250-
responseType: "code",
1251-
state: authorizationUrl.searchParams.get("state"),
1252-
returnTo: "/"
1253-
});
1250+
expect((await decrypt(transactionCookie!.value, secret)).payload).toEqual(
1251+
{
1252+
nonce: authorizationUrl.searchParams.get("nonce"),
1253+
maxAge: 3600,
1254+
codeVerifier: expect.any(String),
1255+
responseType: "code",
1256+
state: authorizationUrl.searchParams.get("state"),
1257+
returnTo: "/"
1258+
}
1259+
);
12541260
});
12551261

12561262
it("should store the returnTo path in the transaction state", async () => {
@@ -1288,13 +1294,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
12881294
`__txn_${authorizationUrl.searchParams.get("state")}`
12891295
);
12901296
expect(transactionCookie).toBeDefined();
1291-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1292-
nonce: authorizationUrl.searchParams.get("nonce"),
1293-
codeVerifier: expect.any(String),
1294-
responseType: "code",
1295-
state: authorizationUrl.searchParams.get("state"),
1296-
returnTo: "https://example.com/dashboard"
1297-
});
1297+
expect((await decrypt(transactionCookie!.value, secret)).payload).toEqual(
1298+
{
1299+
nonce: authorizationUrl.searchParams.get("nonce"),
1300+
codeVerifier: expect.any(String),
1301+
responseType: "code",
1302+
state: authorizationUrl.searchParams.get("state"),
1303+
returnTo: "https://example.com/dashboard"
1304+
}
1305+
);
12981306
});
12991307

13001308
it("should prevent open redirects originating from the returnTo parameter", async () => {
@@ -1332,13 +1340,15 @@ ca/T0LLtgmbMmxSv/MmzIg==
13321340
`__txn_${authorizationUrl.searchParams.get("state")}`
13331341
);
13341342
expect(transactionCookie).toBeDefined();
1335-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1336-
nonce: authorizationUrl.searchParams.get("nonce"),
1337-
codeVerifier: expect.any(String),
1338-
responseType: "code",
1339-
state: authorizationUrl.searchParams.get("state"),
1340-
returnTo: "/"
1341-
});
1343+
expect((await decrypt(transactionCookie!.value, secret)).payload).toEqual(
1344+
{
1345+
nonce: authorizationUrl.searchParams.get("nonce"),
1346+
codeVerifier: expect.any(String),
1347+
responseType: "code",
1348+
state: authorizationUrl.searchParams.get("state"),
1349+
returnTo: "/"
1350+
}
1351+
);
13421352
});
13431353

13441354
describe("with pushed authorization requests", async () => {
@@ -1463,7 +1473,9 @@ ca/T0LLtgmbMmxSv/MmzIg==
14631473
const transactionCookie = transactionCookies[0];
14641474
const state = transactionCookie.name.replace("__txn_", "");
14651475
expect(transactionCookie).toBeDefined();
1466-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1476+
expect(
1477+
(await decrypt(transactionCookie!.value, secret)).payload
1478+
).toEqual({
14671479
nonce: expect.any(String),
14681480
codeVerifier: expect.any(String),
14691481
responseType: "code",
@@ -1540,7 +1552,9 @@ ca/T0LLtgmbMmxSv/MmzIg==
15401552
const transactionCookie = transactionCookies[0];
15411553
const state = transactionCookie.name.replace("__txn_", "");
15421554
expect(transactionCookie).toBeDefined();
1543-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1555+
expect(
1556+
(await decrypt(transactionCookie!.value, secret)).payload
1557+
).toEqual({
15441558
nonce: expect.any(String),
15451559
codeVerifier: expect.any(String),
15461560
responseType: "code",
@@ -1618,7 +1632,9 @@ ca/T0LLtgmbMmxSv/MmzIg==
16181632
const transactionCookie = transactionCookies[0];
16191633
const state = transactionCookie.name.replace("__txn_", "");
16201634
expect(transactionCookie).toBeDefined();
1621-
expect(await decrypt(transactionCookie!.value, secret)).toEqual({
1635+
expect(
1636+
(await decrypt(transactionCookie!.value, secret)).payload
1637+
).toEqual({
16221638
nonce: expect.any(String),
16231639
codeVerifier: expect.any(String),
16241640
responseType: "code",
@@ -2122,7 +2138,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
21222138
// validate the session cookie
21232139
const sessionCookie = response.cookies.get("__session");
21242140
expect(sessionCookie).toBeDefined();
2125-
const session = await decrypt(sessionCookie!.value, secret);
2141+
const { payload: session } = await decrypt(sessionCookie!.value, secret);
21262142
expect(session).toEqual({
21272143
user: {
21282144
sub: DEFAULT.sub
@@ -2230,7 +2246,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
22302246
// validate the session cookie
22312247
const sessionCookie = response.cookies.get("__session");
22322248
expect(sessionCookie).toBeDefined();
2233-
const session = await decrypt(sessionCookie!.value, secret);
2249+
const { payload: session } = await decrypt(sessionCookie!.value, secret);
22342250
expect(session).toEqual({
22352251
user: {
22362252
sub: DEFAULT.sub
@@ -2601,7 +2617,10 @@ ca/T0LLtgmbMmxSv/MmzIg==
26012617
// validate the session cookie
26022618
const sessionCookie = response.cookies.get("__session");
26032619
expect(sessionCookie).toBeDefined();
2604-
const session = await decrypt(sessionCookie!.value, secret);
2620+
const { payload: session } = await decrypt(
2621+
sessionCookie!.value,
2622+
secret
2623+
);
26052624
expect(session).toEqual(expectedSession);
26062625
});
26072626

@@ -3051,7 +3070,10 @@ ca/T0LLtgmbMmxSv/MmzIg==
30513070
// validate the session cookie
30523071
const sessionCookie = response.cookies.get("__session");
30533072
expect(sessionCookie).toBeDefined();
3054-
const session = await decrypt(sessionCookie!.value, secret);
3073+
const { payload: session } = await decrypt(
3074+
sessionCookie!.value,
3075+
secret
3076+
);
30553077
expect(session).toEqual({
30563078
user: {
30573079
sub: DEFAULT.sub,
@@ -3177,7 +3199,10 @@ ca/T0LLtgmbMmxSv/MmzIg==
31773199
// validate the session cookie
31783200
const sessionCookie = response.cookies.get("__session");
31793201
expect(sessionCookie).toBeDefined();
3180-
const session = await decrypt(sessionCookie!.value, secret);
3202+
const { payload: session } = await decrypt(
3203+
sessionCookie!.value,
3204+
secret
3205+
);
31813206
expect(session).toEqual({
31823207
user: {
31833208
sub: DEFAULT.sub,
@@ -3273,7 +3298,7 @@ ca/T0LLtgmbMmxSv/MmzIg==
32733298

32743299
// validate that the session cookie has been updated
32753300
const updatedSessionCookie = response.cookies.get("__session");
3276-
const updatedSession = await decrypt<SessionData>(
3301+
const { payload: updatedSession } = await decrypt<SessionData>(
32773302
updatedSessionCookie!.value,
32783303
secret
32793304
);

‎src/server/auth-client.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -390,14 +390,15 @@ export class AuthClient {
390390
return this.onCallback(new MissingStateError(), {}, null);
391391
}
392392

393-
const transactionState = await this.transactionStore.get(
393+
const transactionStateCookie = await this.transactionStore.get(
394394
req.cookies,
395395
state
396396
);
397-
if (!transactionState) {
397+
if (!transactionStateCookie) {
398398
return this.onCallback(new InvalidStateError(), {}, null);
399399
}
400400

401+
const transactionState = transactionStateCookie.payload;
401402
const onCallbackCtx: OnCallbackContext = {
402403
returnTo: transactionState.returnTo
403404
};

‎src/server/cookies.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe("encrypt/decrypt", async () => {
1212
const encrypted = await encrypt(payload, secret);
1313
const decrypted = await decrypt(encrypted, secret);
1414

15-
expect(decrypted).toEqual(payload);
15+
expect(decrypted.payload).toEqual(payload);
1616
});
1717

1818
it("should fail to decrypt a payload with the incorrect secret", async () => {

‎src/server/cookies.ts

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,15 @@ const DIGEST = "sha256";
88
const BYTE_LENGTH = 32;
99
const ENCRYPTION_INFO = "JWE CEK";
1010

11-
export async function encrypt(payload: jose.JWTPayload, secret: string) {
11+
export async function encrypt(
12+
payload: jose.JWTPayload,
13+
secret: string,
14+
additionalHeaders?: {
15+
iat: number;
16+
uat: number;
17+
exp: number;
18+
}
19+
) {
1220
const encryptionSecret = await hkdf(
1321
DIGEST,
1422
secret,
@@ -18,7 +26,7 @@ export async function encrypt(payload: jose.JWTPayload, secret: string) {
1826
);
1927

2028
const encryptedCookie = await new jose.EncryptJWT(payload)
21-
.setProtectedHeader({ enc: ENC, alg: ALG })
29+
.setProtectedHeader({ enc: ENC, alg: ALG, ...additionalHeaders })
2230
.encrypt(encryptionSecret);
2331

2432
return encryptedCookie.toString();
@@ -35,7 +43,64 @@ export async function decrypt<T>(cookieValue: string, secret: string) {
3543

3644
const cookie = await jose.jwtDecrypt<T>(cookieValue, encryptionSecret, {});
3745

38-
return cookie.payload;
46+
return cookie;
47+
}
48+
49+
/**
50+
* Derive a signing key from a given secret.
51+
* This method is used solely to migrate signed, legacy cookies to the new encrypted cookie format (v4+).
52+
*/
53+
const signingSecret = (secret: string): Promise<Uint8Array> =>
54+
hkdf("sha256", secret, "", "JWS Cookie Signing", BYTE_LENGTH);
55+
56+
/**
57+
* Verify a signed cookie. If the cookie is valid, the value is returned. Otherwise, undefined is returned.
58+
* This method is used solely to migrate signed, legacy cookies to the new encrypted cookie format (v4+).
59+
*/
60+
export async function verifySigned(
61+
k: string,
62+
v: string,
63+
secret: string
64+
): Promise<string | undefined> {
65+
if (!v) {
66+
return undefined;
67+
}
68+
const [value, signature] = v.split(".");
69+
const flattenedJWS = {
70+
protected: jose.base64url.encode(
71+
JSON.stringify({ alg: "HS256", b64: false, crit: ["b64"] })
72+
),
73+
payload: `${k}=${value}`,
74+
signature
75+
};
76+
const key = await signingSecret(secret);
77+
78+
try {
79+
await jose.flattenedVerify(flattenedJWS, key, {
80+
algorithms: ["HS256"]
81+
});
82+
return value;
83+
} catch (e) {
84+
return undefined;
85+
}
86+
}
87+
88+
/**
89+
* Sign a cookie value using a secret.
90+
* This method is used solely to migrate signed, legacy cookies to the new encrypted cookie format (v4+).
91+
*/
92+
export async function sign(
93+
name: string,
94+
value: string,
95+
secret: string
96+
): Promise<string> {
97+
const key = await signingSecret(secret);
98+
const { signature } = await new jose.FlattenedSign(
99+
new TextEncoder().encode(`${name}=${value}`)
100+
)
101+
.setProtectedHeader({ alg: "HS256", b64: false, crit: ["b64"] })
102+
.sign(key);
103+
return `${value}.${signature}`;
39104
}
40105

41106
export interface CookieOptions {

0 commit comments

Comments
 (0)