Skip to content

Commit af39cb4

Browse files
fix: correctly handle expired JWE's in cookies
1 parent c44432c commit af39cb4

File tree

5 files changed

+118
-41
lines changed

5 files changed

+118
-41
lines changed

src/server/cookies.test.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,8 @@ describe("encrypt/decrypt", async () => {
3131
const payload = { key: "value" };
3232
const expiration = Math.floor(Date.now() / 1000 - 60); // 60 seconds in the past
3333
const encrypted = await encrypt(payload, secret, expiration);
34-
await expect(() =>
35-
decrypt(encrypted, secret)
36-
).rejects.toThrowError(`"exp" claim timestamp check failed`);
34+
const decrypted = await decrypt(encrypted, secret);
35+
expect(decrypted).toBeNull();
3736
});
3837

3938
it("should fail to encrypt if a secret is not provided", async () => {

src/server/cookies.ts

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,17 +39,24 @@ export async function encrypt(
3939
}
4040

4141
export async function decrypt<T>(cookieValue: string, secret: string) {
42-
const encryptionSecret = await hkdf(
43-
DIGEST,
44-
secret,
45-
"",
46-
ENCRYPTION_INFO,
47-
BYTE_LENGTH
48-
);
42+
try {
43+
const encryptionSecret = await hkdf(
44+
DIGEST,
45+
secret,
46+
"",
47+
ENCRYPTION_INFO,
48+
BYTE_LENGTH
49+
);
4950

50-
const cookie = await jose.jwtDecrypt<T>(cookieValue, encryptionSecret, {});
51+
const cookie = await jose.jwtDecrypt<T>(cookieValue, encryptionSecret, {});
5152

52-
return cookie;
53+
return cookie;
54+
} catch (e: any) {
55+
if (e.code === "ERR_JWT_EXPIRED") {
56+
return null;
57+
}
58+
throw e;
59+
}
5360
}
5461

5562
/**

src/server/session/stateful-session-store.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,16 @@ export class StatefulSessionStore extends AbstractSessionStore {
7272
// this ensures that v3 sessions are respected and can be transparently rolled over to v4+ sessions
7373
let sessionId: string | null = null;
7474
try {
75-
const { payload: sessionCookie } =
76-
await cookies.decrypt<SessionCookieValue>(cookie.value, this.secret);
77-
sessionId = sessionCookie.id;
75+
const sessionCookie = await cookies.decrypt<SessionCookieValue>(
76+
cookie.value,
77+
this.secret
78+
);
79+
80+
if (sessionCookie === null) {
81+
return null;
82+
}
83+
84+
sessionId = sessionCookie.payload.id;
7885
} catch (e: any) {
7986
// the session cookie could not be decrypted, try to verify if it's a legacy session
8087
if (e.code === "ERR_JWE_INVALID") {
@@ -115,9 +122,12 @@ export class StatefulSessionStore extends AbstractSessionStore {
115122
let sessionId = null;
116123
const cookieValue = reqCookies.get(this.sessionCookieName)?.value;
117124
if (cookieValue) {
118-
const { payload: sessionCookie } =
125+
const sessionCookie =
119126
await cookies.decrypt<SessionCookieValue>(cookieValue, this.secret);
120-
sessionId = sessionCookie.id;
127+
128+
if (sessionCookie) {
129+
sessionId = sessionCookie.payload.id;
130+
}
121131
}
122132

123133
// if this is a new session created by a new login we need to remove the old session
@@ -130,17 +140,17 @@ export class StatefulSessionStore extends AbstractSessionStore {
130140
if (!sessionId) {
131141
sessionId = generateId();
132142
}
133-
143+
134144
const maxAge = this.calculateMaxAge(session.internal.createdAt);
135145
const expiration = Date.now() / 1000 + maxAge;
136146
const jwe = await cookies.encrypt(
137147
{
138148
id: sessionId
139149
},
140150
this.secret,
141-
expiration,
151+
expiration
142152
);
143-
153+
144154
resCookies.set(this.sessionCookieName, jwe.toString(), {
145155
...this.cookieConfig,
146156
maxAge
@@ -168,11 +178,13 @@ export class StatefulSessionStore extends AbstractSessionStore {
168178
return;
169179
}
170180

171-
const { payload: session } = await cookies.decrypt<SessionCookieValue>(
181+
const session = await cookies.decrypt<SessionCookieValue>(
172182
cookieValue,
173183
this.secret
174184
);
175185

176-
await this.store.delete(session.id);
186+
if (session) {
187+
await this.store.delete(session.payload.id);
188+
}
177189
}
178190
}

src/server/session/stateless-session-store.test.ts

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -209,27 +209,75 @@ describe("Stateless Session Store", async () => {
209209
sid: "auth0-sid",
210210
createdAt: Math.floor(Date.now() / 1000)
211211
},
212-
federatedConnectionTokenSets: [
213-
{
214-
connection: "google-oauth",
215-
accessToken: "google-at-123",
216-
expiresAt: 123456
217-
}
218-
]
212+
};
213+
214+
const googleConnectionTokenSet = {
215+
connection: "google-oauth",
216+
accessToken: "google-at-123",
217+
expiresAt: 123456
219218
};
220219
const maxAge = 60 * 60; // 1 hour in seconds
221220
const expiration = Math.floor(Date.now() / 1000 + maxAge);
222221
const encryptedCookieValue = await encrypt(session, secret, expiration);
222+
const encryptedGoogleConnectionCookieValue = await encrypt(googleConnectionTokenSet, secret, expiration);
223223

224224
const headers = new Headers();
225-
headers.append("cookie", `__session=${encryptedCookieValue}`);
225+
headers.append("cookie", `__session=${encryptedCookieValue};__FC.0=${encryptedGoogleConnectionCookieValue}`);
226226
const requestCookies = new RequestCookies(headers);
227227

228228
const sessionStore = new StatelessSessionStore({
229229
secret
230230
});
231231

232-
expect(await sessionStore.get(requestCookies)).toEqual(expect.objectContaining(session));
232+
const result = await sessionStore.get(requestCookies);
233+
234+
expect(result).toEqual(expect.objectContaining(session));
235+
expect(result?.connectionTokenSets).toEqual([expect.objectContaining(googleConnectionTokenSet)]);
236+
});
237+
238+
it("should return the decrypted session cookie if it exists and exclude a connection when the JWE is expired", async () => {
239+
const secret = await generateSecret(32);
240+
const session: SessionData = {
241+
user: { sub: "user_123" },
242+
tokenSet: {
243+
accessToken: "at_123",
244+
refreshToken: "rt_123",
245+
expiresAt: 123456
246+
},
247+
internal: {
248+
sid: "auth0-sid",
249+
createdAt: Math.floor(Date.now() / 1000)
250+
},
251+
};
252+
253+
const googleConnectionTokenSet = {
254+
connection: "google-oauth",
255+
accessToken: "google-at-123",
256+
expiresAt: 123456
257+
};
258+
const githubConnectionTokenSet = {
259+
connection: "github",
260+
accessToken: "github-at-123",
261+
expiresAt: 123456
262+
};
263+
const maxAge = 60 * 60; // 1 hour in seconds
264+
const expiration = Math.floor(Date.now() / 1000 + maxAge);
265+
const encryptedCookieValue = await encrypt(session, secret, expiration);
266+
const encryptedGoogleConnectionCookieValue = await encrypt(googleConnectionTokenSet, secret, Math.floor(Date.now() / 1000 - 10)); // expired
267+
const encryptedGithubConnectionCookieValue = await encrypt(githubConnectionTokenSet, secret, expiration);
268+
269+
const headers = new Headers();
270+
headers.append("cookie", `__session=${encryptedCookieValue};__FC.0=${encryptedGoogleConnectionCookieValue};__FC.1=${encryptedGithubConnectionCookieValue}`);
271+
const requestCookies = new RequestCookies(headers);
272+
273+
const sessionStore = new StatelessSessionStore({
274+
secret
275+
});
276+
277+
const result = await sessionStore.get(requestCookies);
278+
279+
expect(result).toEqual(expect.objectContaining(session));
280+
expect(result?.connectionTokenSets).toEqual([expect.objectContaining(githubConnectionTokenSet)]);
233281
});
234282
});
235283

@@ -320,9 +368,8 @@ describe("Stateless Session Store", async () => {
320368

321369
expect(cookie).toBeDefined();
322370

323-
await expect(
324-
decrypt(cookie!.value, secret)
325-
).rejects.toThrow(`"exp" claim timestamp check failed`);
371+
const decryptedSession = await decrypt(cookie!.value, secret);
372+
expect(decryptedSession).toEqual(null);
326373
});
327374

328375
it("should delete the legacy cookie if it exists", async () => {

src/server/session/stateless-session-store.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,25 +54,37 @@ export class StatelessSessionStore extends AbstractSessionStore {
5454
SessionData | LegacySessionPayload
5555
>(cookieValue, this.secret);
5656

57+
if (!originalSession) {
58+
return null;
59+
}
60+
5761
const normalizedStatelessSession =
5862
normalizeStatelessSession(originalSession);
5963

6064
// As connection access tokens are stored in seperate cookies,
6165
// we need to get all cookies and only use those that are prefixed with `this.connectionTokenSetsCookieName`
62-
const connectionTokenSets = await Promise.all(
63-
this.getConnectionTokenSetsCookies(reqCookies).map((cookie) =>
64-
cookies.decrypt<ConnectionTokenSet>(cookie.value, this.secret)
65-
)
66+
const connectionTokenSetsCookies = this.getConnectionTokenSetsCookies(
67+
reqCookies
6668
);
6769

70+
const connectionTokenSets = [];
71+
for (const cookie of connectionTokenSetsCookies) {
72+
const decryptedCookie = await cookies.decrypt<ConnectionTokenSet>(
73+
cookie.value,
74+
this.secret
75+
);
76+
77+
if (decryptedCookie) {
78+
connectionTokenSets.push(decryptedCookie.payload);
79+
}
80+
}
81+
6882
return {
6983
...normalizedStatelessSession,
7084
// Ensure that when there are no connection token sets, we omit the property.
7185
...(connectionTokenSets.length
7286
? {
73-
connectionTokenSets: connectionTokenSets.map(
74-
(tokenSet) => tokenSet.payload
75-
)
87+
connectionTokenSets
7688
}
7789
: {})
7890
};

0 commit comments

Comments
 (0)