diff --git a/.changeset/destory-session-maxage.md b/.changeset/destory-session-maxage.md new file mode 100644 index 00000000000..660b51c0dac --- /dev/null +++ b/.changeset/destory-session-maxage.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Fix `destroySession` for sessions using a `maxAge` cookie. The data in the cookie was always properly destroyed but when using `maxAge`, the cookie itself wasn't deleted because `Max-Age` takes precedence over `Expires` in the cookie spec. diff --git a/packages/remix-server-runtime/__tests__/sessions-test.ts b/packages/remix-server-runtime/__tests__/sessions-test.ts index 7b06e538d66..041b5deb75c 100644 --- a/packages/remix-server-runtime/__tests__/sessions-test.ts +++ b/packages/remix-server-runtime/__tests__/sessions-test.ts @@ -150,6 +150,37 @@ describe("Cookie session storage", () => { await expect(() => commitSession(session)).rejects.toThrow(); }); + it("destroys sessions using a past date", async () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let { getSession, destroySession } = createCookieSessionStorage({ + cookie: { + secrets: ["secret1"], + }, + }); + let session = await getSession(); + let setCookie = await destroySession(session); + expect(setCookie).toMatchInlineSnapshot( + `"__session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax"` + ); + spy.mockRestore(); + }); + + it("destroys sessions that leverage maxAge", async () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let { getSession, destroySession } = createCookieSessionStorage({ + cookie: { + maxAge: 60 * 60, // 1 hour + secrets: ["secret1"], + }, + }); + let session = await getSession(); + let setCookie = await destroySession(session); + expect(setCookie).toMatchInlineSnapshot( + `"__session=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax"` + ); + spy.mockRestore(); + }); + describe("warnings when providing options you may not want to", () => { let spy = spyConsole(); @@ -207,6 +238,104 @@ describe("Cookie session storage", () => { }); }); +describe("Custom cookie-backed session storage", () => { + let memoryBacking = {}; + let createCookieBackedSessionStorage = + createSessionStorageFactory(createCookie); + let implementation = { + createData(data) { + let id = Math.random().toString(36).substring(2, 10); + memoryBacking[id] = data; + return Promise.resolve(id); + }, + readData(id) { + return Promise.resolve(memoryBacking[id] || null); + }, + updateData(id, data) { + memoryBacking[id] = data; + return Promise.resolve(); + }, + deleteData(id) { + memoryBacking[id] = null; + return Promise.resolve(memoryBacking[id]); + }, + }; + + it("persists session data across requests", async () => { + let { getSession, commitSession } = createCookieBackedSessionStorage({ + ...implementation, + cookie: createCookie("test", { secrets: ["test"] }), + }); + let session = await getSession(); + session.set("user", "mjackson"); + let setCookie = await commitSession(session); + session = await getSession(getCookieFromSetCookie(setCookie)); + + expect(session.get("user")).toEqual("mjackson"); + }); + + it("returns an empty session for cookies that are not signed properly", async () => { + let { getSession, commitSession } = createCookieBackedSessionStorage({ + ...implementation, + cookie: createCookie("test", { secrets: ["test"] }), + }); + let session = await getSession(); + session.set("user", "mjackson"); + + expect(session.get("user")).toEqual("mjackson"); + + let setCookie = await commitSession(session); + session = await getSession( + // Tamper with the session cookie... + getCookieFromSetCookie(setCookie).slice(0, -1) + ); + + expect(session.get("user")).toBeUndefined(); + }); + + it('"makes the default path of cookies to be /', async () => { + let { getSession, commitSession } = createCookieBackedSessionStorage({ + ...implementation, + cookie: createCookie("test", { secrets: ["test"] }), + }); + let session = await getSession(); + session.set("user", "mjackson"); + let setCookie = await commitSession(session); + expect(setCookie).toContain("Path=/"); + }); + + it("destroys sessions using a past date", async () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let { getSession, destroySession } = createCookieBackedSessionStorage({ + ...implementation, + cookie: createCookie("test", { secrets: ["test"] }), + }); + let session = await getSession(); + let setCookie = await destroySession(session); + expect(setCookie).toMatchInlineSnapshot( + `"test=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax"` + ); + spy.mockRestore(); + }); + + it("destroys sessions that leverage maxAge", async () => { + let spy = jest.spyOn(console, "warn").mockImplementation(() => {}); + let { getSession, destroySession } = createCookieBackedSessionStorage({ + ...implementation, + cookie: createCookie("test", { + maxAge: 60 * 60, // 1 hour + secrets: ["test"], + }), + }); + let session = await getSession(); + let setCookie = await destroySession(session); + expect(setCookie).toMatchInlineSnapshot( + `"test=; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; SameSite=Lax"` + ); + spy.mockRestore(); + }); +}); + function spyConsole() { // https://github.com/facebook/react/issues/7047 let spy: any = {}; diff --git a/packages/remix-server-runtime/sessions.ts b/packages/remix-server-runtime/sessions.ts index 2d4a6c90806..c00eedb2321 100644 --- a/packages/remix-server-runtime/sessions.ts +++ b/packages/remix-server-runtime/sessions.ts @@ -296,6 +296,7 @@ export const createSessionStorageFactory = await deleteData(session.id); return cookie.serialize("", { ...options, + maxAge: undefined, expires: new Date(0), }); }, diff --git a/packages/remix-server-runtime/sessions/cookieStorage.ts b/packages/remix-server-runtime/sessions/cookieStorage.ts index 9cf4766da7f..09a2e765681 100644 --- a/packages/remix-server-runtime/sessions/cookieStorage.ts +++ b/packages/remix-server-runtime/sessions/cookieStorage.ts @@ -61,6 +61,7 @@ export const createCookieSessionStorageFactory = async destroySession(_session, options) { return cookie.serialize("", { ...options, + maxAge: undefined, expires: new Date(0), }); },