Skip to content

Commit

Permalink
Remove session args from Keystone constructor (#5198)
Browse files Browse the repository at this point in the history
  • Loading branch information
timleslie authored Mar 23, 2021
1 parent 32578f0 commit b36758a
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 66 deletions.
8 changes: 8 additions & 0 deletions .changeset/smart-eyes-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@keystone-next/keystone-legacy': major
'@keystone-next/keystone': patch
'@keystone-next/session-legacy': patch
'@keystone-next/test-utils-legacy': patch
---

Removed the legacy `cookieSecret`, `cookie`, and `sessionStore` arguments from the `Keystone` constructor.
3 changes: 0 additions & 3 deletions packages-next/keystone/src/lib/createKeystone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,12 @@ export function createKeystone(
// @ts-ignore The @types/keystonejs__keystone package has the wrong type for KeystoneOptions
const keystone: BaseKeystone = new Keystone({
adapter,
cookieSecret: '123456789', // FIXME: Don't provide a default here. See #2882
queryLimits: graphql?.queryLimits,
// @ts-ignore The @types/keystonejs__keystone package has the wrong type for KeystoneOptions
onConnect: (keystone, { context } = {}) => config.db.onConnect?.(context),
// FIXME: Unsupported options: Need to work which of these we want to support with backwards
// compatibility options.
// defaultAccess
// sessionStore
// cookie
// schemaNames
});

Expand Down
28 changes: 0 additions & 28 deletions packages/keystone/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,13 @@ const { Keystone } = require('@keystone-next/keystone-legacy');

const keystone = new Keystone({
adapter,
cookie,
cookieSecret,
defaultAccess,
onConnect,
queryLimits,
sessionStore,
schemaNames,
});
```

### `cookie`

_**Default:**_ see Usage.

A description of the cookie properties is included in the [express-session documentation](https://github.com/expressjs/session#cookie).

#### `secure`

A secure cookie is only sent to the server with an encrypted request over the HTTPS protocol. If `secure` is set to true (as is the default with a **production** build) for a KeystoneJS project running on a non-HTTPS server (such as localhost), you will **not** be able to log in. In that case, be sure you set `secure` to false. This does not affect development builds since this value is already false.
Expand All @@ -48,10 +39,6 @@ const keystone = new Keystone({
});
```

### `cookieSecret`

The secret used to sign session ID cookies. In production mode (`process.env.NODE_ENV === 'production'`) this option is required. In development mode, if undefined, a random `cookieSecret` will be generated each time Keystone starts (this will cause sessions to be reset between restarts).

### `defaultAccess`

_**Default:**_
Expand Down Expand Up @@ -90,21 +77,6 @@ const keystone = new Keystone({

Note that `maxTotalResults` applies to the total results of all relationship queries separately, even if some are nested inside others.

### `sessionStore`

Sets the Express server's [session middleware](https://github.com/expressjs/session). This should be configured before deploying your app.

This example uses the [`connect-mongo`](https://github.com/jdesboeufs/connect-mongo) middleware, but you can use [any of the stores that work with `express session`](https://github.com/expressjs/session#compatible-session-stores).

```javascript
const expressSession = require('express-session');
const MongoStore = require('connect-mongo')(expressSession);

const keystone = new Keystone({
sessionStore: new MongoStore({ url: 'mongodb://localhost/my-app' }),
});
```

### `schemaNames`

_**Default:**_ `['public']`
Expand Down
23 changes: 6 additions & 17 deletions packages/keystone/lib/Keystone/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,19 @@ const { formatError } = require('./format-error');
const composePlugins = fns => (o, e) => fns.reduce((acc, fn) => fn(acc, e), o);

module.exports = class Keystone {
constructor({
defaultAccess,
adapter,
onConnect,
cookieSecret,
sessionStore,
queryLimits = {},
cookie = {
secure: process.env.NODE_ENV === 'production', // Default to true in production
maxAge: 1000 * 60 * 60 * 24 * 30, // 30 days
sameSite: false,
},
schemaNames = ['public'],
}) {
constructor({ defaultAccess, adapter, onConnect, queryLimits = {}, schemaNames = ['public'] }) {
this.defaultAccess = { list: true, field: true, custom: true, ...defaultAccess };
this.auth = {};
this.lists = {};
this.listsArray = [];
this.getListByKey = key => this.lists[key];
this._schemas = {};
this._sessionManager = new SessionManager({
cookieSecret,
cookie,
sessionStore,
cookie: {
secure: process.env.NODE_ENV === 'production', // Default to true in production
maxAge: 1000 * 60 * 60 * 24 * 30, // 30 days
sameSite: false,
},
});
this.eventHandlers = { onConnect };
this.registeredTypes = new Set();
Expand Down
13 changes: 0 additions & 13 deletions packages/keystone/tests/Keystone.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ describe('Keystone.createList()', () => {
test('basic', () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);

Expand All @@ -92,7 +91,6 @@ describe('Keystone.createList()', () => {
test('Reserved words', () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);

Expand All @@ -114,7 +112,6 @@ describe('Keystone.createList()', () => {
test('plugins', () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);

Expand Down Expand Up @@ -159,7 +156,6 @@ describe('keystone.prepare()', () => {
test('returns a Promise', () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);

Expand All @@ -171,7 +167,6 @@ describe('keystone.prepare()', () => {
test('returns the middlewares array', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
const { middlewares } = await keystone.prepare();
Expand All @@ -182,7 +177,6 @@ describe('keystone.prepare()', () => {
test('handles apps:undefined', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
const { middlewares } = await keystone.prepare({ apps: undefined });
Expand All @@ -193,7 +187,6 @@ describe('keystone.prepare()', () => {
test('handles apps:[]', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
const { middlewares } = await keystone.prepare({ apps: [] });
Expand All @@ -204,7 +197,6 @@ describe('keystone.prepare()', () => {
test('Handles apps without a `prepareMiddleware`', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
// For less-brittle tests, we grab the list of middlewares when prepare is
Expand All @@ -219,7 +211,6 @@ describe('keystone.prepare()', () => {
test('filters out null middleware results', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
// For less-brittle tests, we grab the list of middlewares when prepare is
Expand All @@ -234,7 +225,6 @@ describe('keystone.prepare()', () => {
test('filters out empty middleware arrays', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
// For less-brittle tests, we grab the list of middlewares when prepare is
Expand All @@ -249,7 +239,6 @@ describe('keystone.prepare()', () => {
test('returns middlewares', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const middleware = jest.fn(() => {});
const keystone = new Keystone(config);
Expand All @@ -264,7 +253,6 @@ describe('keystone.prepare()', () => {
test('flattens deeply nested middlewares', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);
const fn0 = jest.fn(() => {});
Expand All @@ -281,7 +269,6 @@ describe('keystone.prepare()', () => {
test('should create `internal` GraphQL schema instance', async () => {
const config = {
adapter: new MockAdapter(),
cookieSecret: 'secretForTesting',
};
const keystone = new Keystone(config);

Expand Down
4 changes: 0 additions & 4 deletions packages/session/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ export class SessionManager {
'The cookieSecret config option is required when running Keystone in a production environment. Update your app or environment config so this value is supplied to the Keystone constructor. See [https://www.keystonejs.com/keystonejs/keystone/#cookiesecret] for details.'
);
} else {
console.warn(
'No cookieSecret value was provided. Please generate a secure value and add it to your app. Until this is done, a random cookieSecret will be generated each time Keystone is started. This will cause sessions to be reset between restarts. See [https://www.keystonejs.com/keystonejs/keystone/#cookiesecret] for details.'
);

cookieSecret = [...Array(30)].map(() => ((Math.random() * 36) | 0).toString(36)).join('');
}
}
Expand Down
1 change: 0 additions & 1 deletion packages/test-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@ async function setupServer({
// @ts-ignore The @types/keystonejs__keystone package has the wrong type for KeystoneOptions
defaultAccess: { list: true, field: true },
schemaNames,
cookieSecret: 'secretForTesting',
...keystoneOptions,
});

Expand Down

1 comment on commit b36758a

@vercel
Copy link

@vercel vercel bot commented on b36758a Mar 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.