Skip to content

Commit

Permalink
chore(remix-server-runtime): warn against using expires in `createC…
Browse files Browse the repository at this point in the history
…ookieSessionStorage` and `createSessionStorage` (remix-run#3091)

Co-authored-by: Michaël De Boey <info@michaeldeboey.be>
Co-authored-by: Michael Jackson <michael@jackson.us>
  • Loading branch information
3 people authored May 18, 2022
1 parent b4fb6c5 commit 514c808
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 0 deletions.
31 changes: 31 additions & 0 deletions packages/remix-server-runtime/__tests__/cookies-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,35 @@ describe("cookies", () => {
});
expect(setCookie2).toContain("Path=/about");
});

describe("warnings when providing options you may not want to", () => {
let spy = spyConsole();

it("warns against using `expires` when creating the cookie instance", async () => {
createCookie("my-cookie", { expires: new Date(Date.now() + 60_000) });
expect(spy.console).toHaveBeenCalledTimes(1);
expect(spy.console).toHaveBeenCalledWith(
'The "my-cookie" cookie has an "expires" property set. This will cause the expires value to not be updated when the session is committed. Instead, you should set the expires value when serializing the cookie. You can use `commitSession(session, { expires })` if using a session storage object, or `cookie.serialize("value", { expires })` if you\'re using the cookie directly.'
);
});
});
});

function spyConsole() {
// https://github.com/facebook/react/issues/7047
let spy: any = {};

beforeAll(() => {
spy.console = jest.spyOn(console, "warn").mockImplementation(() => {});
});

beforeEach(() => {
spy.console.mockClear();
});

afterAll(() => {
spy.console.mockRestore();
});

return spy;
}
46 changes: 46 additions & 0 deletions packages/remix-server-runtime/__tests__/sessions-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,33 @@ describe("Cookie session storage", () => {
await expect(() => commitSession(session)).rejects.toThrow();
});

describe("warnings when providing options you may not want to", () => {
let spy = spyConsole();

it("warns against using `expires` when creating the session", async () => {
createCookieSessionStorage({
cookie: {
secrets: ["secret1"],
expires: new Date(Date.now() + 60_000),
},
});

expect(spy.console).toHaveBeenCalledTimes(1);
expect(spy.console).toHaveBeenCalledWith(
'The "__session" cookie has an "expires" property set. This will cause the expires value to not be updated when the session is committed. Instead, you should set the expires value when serializing the cookie. You can use `commitSession(session, { expires })` if using a session storage object, or `cookie.serialize("value", { expires })` if you\'re using the cookie directly.'
);
});

it("warns when not passing secrets when creating the session", async () => {
createCookieSessionStorage({ cookie: {} });

expect(spy.console).toHaveBeenCalledTimes(1);
expect(spy.console).toHaveBeenCalledWith(
'The "__session" cookie is not signed, but session cookies should be signed to prevent tampering on the client before they are sent back to the server. See https://remix.run/api/remix#signing-cookies for more information.'
);
});
});

describe("when a new secret shows up in the rotation", () => {
it("unsigns old session cookies using the old secret and encodes new cookies using the new secret", async () => {
let { getSession, commitSession } = createCookieSessionStorage({
Expand Down Expand Up @@ -168,3 +195,22 @@ describe("Cookie session storage", () => {
});
});
});

function spyConsole() {
// https://github.com/facebook/react/issues/7047
let spy: any = {};

beforeAll(() => {
spy.console = jest.spyOn(console, "warn").mockImplementation(() => {});
});

beforeEach(() => {
spy.console.mockClear();
});

afterAll(() => {
spy.console.mockRestore();
});

return spy;
}
14 changes: 14 additions & 0 deletions packages/remix-server-runtime/cookies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { CookieParseOptions, CookieSerializeOptions } from "cookie";
import { parse, serialize } from "cookie";

import type { SignFunction, UnsignFunction } from "./crypto";
import { warnOnce } from "./warnings";

export type { CookieParseOptions, CookieSerializeOptions };

Expand Down Expand Up @@ -91,6 +92,8 @@ export const createCookieFactory =
...cookieOptions,
};

warnOnceAboutExpiresCookie(name, options.expires);

return {
get name() {
return name;
Expand Down Expand Up @@ -245,3 +248,14 @@ function myUnescape(value: string): string {
}
return result;
}

function warnOnceAboutExpiresCookie(name: string, expires?: Date) {
warnOnce(
!expires,
`The "${name}" cookie has an "expires" property set. ` +
`This will cause the expires value to not be updated when the session is committed. ` +
`Instead, you should set the expires value when serializing the cookie. ` +
`You can use \`commitSession(session, { expires })\` if using a session storage object, ` +
`or \`cookie.serialize("value", { expires })\` if you're using the cookie directly.`
);
}

0 comments on commit 514c808

Please sign in to comment.