Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 227 - allow custom session id generation #252

Merged
merged 2 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,16 @@ interface SessionConfigParams {
*/
store?: SessionStore;

/**
* A Function for generating a session id when using a custom session store.
* For full details see the documentation for express-session
* at [genid](https://github.com/expressjs/session/blob/master/README.md#genid).
* If encrypted cookie storage is used or no value is provided, a default implementation is used.
* Be aware the default implmentation is slightly different in this library as compared to the
* default session id generation used express-session.
*/
genid?: (req: OpenidRequest) => string;

/**
* If you want your session duration to be rolling, eg reset everytime the
* user is active on your site, set this to a `true`. If you want the session
Expand Down
18 changes: 11 additions & 7 deletions lib/appSession.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const {
JWE,
errors: { JOSEError },
} = require('jose');
const crypto = require('crypto');
const { promisify } = require('util');
const cookie = require('cookie');
const onHeaders = require('on-headers');
Expand Down Expand Up @@ -44,15 +43,20 @@ module.exports = (config) => {
const sessionName = config.session.name;
const cookieConfig = config.session.cookie;
const {
genid: generateId,
absoluteDuration,
rolling: rollingEnabled,
rollingDuration,
} = config.session;

const { transient: emptyTransient , ...emptyCookieOptions } = cookieConfig;
const { transient: emptyTransient, ...emptyCookieOptions } = cookieConfig;
emptyCookieOptions.expires = emptyTransient ? 0 : new Date();

const emptyCookie = cookie.serialize(`${sessionName}.0`, '', emptyCookieOptions);
const emptyCookie = cookie.serialize(
`${sessionName}.0`,
'',
emptyCookieOptions
);
const cookieChunkSize = MAX_COOKIE_SIZE - emptyCookie.length;

let keystore = new JWKS.KeyStore();
Expand Down Expand Up @@ -97,7 +101,7 @@ module.exports = (config) => {
{ uat = epoch(), iat = uat, exp = calculateExp(iat, uat) }
) {
const cookies = req[COOKIES];
const { transient: cookieTransient , ...cookieOptions } = cookieConfig;
const { transient: cookieTransient, ...cookieOptions } = cookieConfig;
cookieOptions.expires = cookieTransient ? 0 : new Date(exp * 1000);

// session was deleted or is empty, this matches all session cookies (chunked or unchunked)
Expand Down Expand Up @@ -143,7 +147,7 @@ module.exports = (config) => {
debug('replacing non chunked cookie with chunked cookies');
res.clearCookie(sessionName, {
domain: cookieConfig.domain,
path: cookieConfig.path
path: cookieConfig.path,
});
}
} else {
Expand All @@ -153,7 +157,7 @@ module.exports = (config) => {
if (cookieName.match(`^${sessionName}\\.\\d$`)) {
res.clearCookie(cookieName, {
domain: cookieConfig.domain,
path: cookieConfig.path
path: cookieConfig.path,
});
}
}
Expand Down Expand Up @@ -325,7 +329,7 @@ module.exports = (config) => {
attachSessionObject(req, sessionName, {});
}

const id = existingSessionValue || crypto.randomBytes(16).toString('hex');
const id = existingSessionValue || generateId(req);

onHeaders(res, () => store.setCookie(id, req, res, { iat }));

Expand Down
7 changes: 7 additions & 0 deletions lib/config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
const Joi = require('joi');
const crypto = require('crypto');
const { defaultState: getLoginState } = require('./hooks/getLoginState');
const isHttps = /^https:/i;

const defaultSessionIdGenerator = () => crypto.randomBytes(16).toString('hex');

const paramsSchema = Joi.object({
secret: Joi.alternatives([
Joi.string().min(8),
Expand Down Expand Up @@ -38,6 +41,10 @@ const paramsSchema = Joi.object({
.default(7 * 24 * 60 * 60), // 7 days,
name: Joi.string().token().optional().default('appSession'),
store: Joi.object().optional(),
genid: Joi.function()
.maxArity(1)
.optional()
.default(() => defaultSessionIdGenerator),
cookie: Joi.object({
domain: Joi.string().optional(),
transient: Joi.boolean().optional().default(false),
Expand Down
27 changes: 26 additions & 1 deletion test/appSession.customStore.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ describe('appSession custom store', () => {

const conf = getConfig({
...defaultConfig,
session: { store, ...(config && config.session) },
...config,
session: { ...(config && config.session), store },
});

server = await createServer(appSession(conf));
Expand Down Expand Up @@ -155,6 +155,31 @@ describe('appSession custom store', () => {
assert.isEmpty(jar.getCookies(baseUrl));
});

it('uses custom session id generator when provided', async () => {
const immId = 'apple';
await setup({
session: { genid: () => immId },
});
const jar = await login({
sub: '__foo_user__',
role: 'test',
userid: immId,
});
const res = await request.get('/session', {
baseUrl,
jar,
json: true,
});
assert.equal(res.statusCode, 200);
const storedSessionJson = await redisClient.asyncGet(immId);
const { data: sessionValues } = JSON.parse(storedSessionJson);
assert.deepEqual(sessionValues, {
sub: '__foo_user__',
role: 'test',
userid: immId,
});
});

it('should handle storage errors', async () => {
const store = {
get(id, cb) {
Expand Down
80 changes: 62 additions & 18 deletions test/appSession.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,10 @@ describe('appSession', () => {

it('should limit total cookie size to 4096 Bytes', async () => {
const path =
'/some-really-really-really-really-really-really-really-really-really-really-really-really-really-long-path';
server = await createServer(appSession(getConfig({ ...defaultConfig, session: { cookie: { path } } })));
'/some-really-really-really-really-really-really-really-really-really-really-really-really-really-long-path';
server = await createServer(
appSession(getConfig({ ...defaultConfig, session: { cookie: { path } } }))
);
const jar = request.jar();

await request.post('session', {
Expand All @@ -139,24 +141,30 @@ describe('appSession', () => {

const cookies = jar
.getCookies(`${baseUrl}${path}`)
.reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
.reduce(
(obj, value) => Object.assign(obj, { [value.key]: value + '' }),
{}
);

assert.exists(cookies);
assert.equal(cookies['appSession.0'].length, 4096);
assert.equal(cookies['appSession.1'].length, 4096);
assert.equal(cookies['appSession.2'].length, 4096);
assert.isTrue(cookies['appSession.3'].length <= 4096)
assert.isTrue(cookies['appSession.3'].length <= 4096);
});

it('should clean up single cookie when switching to chunked', async () => {
server = await createServer(appSession(getConfig(defaultConfig)));
const jar = request.jar();
jar.setCookie(`appSession=foo`, baseUrl)
jar.setCookie(`appSession=foo`, baseUrl);

const firstCookies = jar
.getCookies(baseUrl)
.reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
assert.property(firstCookies, 'appSession')
.reduce(
(obj, value) => Object.assign(obj, { [value.key]: value + '' }),
{}
);
assert.property(firstCookies, 'appSession');

await request.post('session', {
baseUrl,
Expand All @@ -169,23 +177,29 @@ describe('appSession', () => {

const cookies = jar
.getCookies(baseUrl)
.reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
.reduce(
(obj, value) => Object.assign(obj, { [value.key]: value + '' }),
{}
);

assert.property(cookies, 'appSession.0')
assert.notProperty(cookies, 'appSession')
assert.property(cookies, 'appSession.0');
assert.notProperty(cookies, 'appSession');
});

it('should clean up chunked cookies when switching to single cookie', async () => {
server = await createServer(appSession(getConfig(defaultConfig)));
const jar = request.jar();
jar.setCookie(`appSession.0=foo`, baseUrl)
jar.setCookie(`appSession.1=foo`, baseUrl)
jar.setCookie(`appSession.0=foo`, baseUrl);
jar.setCookie(`appSession.1=foo`, baseUrl);

const firstCookies = jar
.getCookies(baseUrl)
.reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
assert.property(firstCookies, 'appSession.0')
assert.property(firstCookies, 'appSession.1')
.reduce(
(obj, value) => Object.assign(obj, { [value.key]: value + '' }),
{}
);
assert.property(firstCookies, 'appSession.0');
assert.property(firstCookies, 'appSession.1');

await request.post('session', {
baseUrl,
Expand All @@ -197,10 +211,13 @@ describe('appSession', () => {

const cookies = jar
.getCookies(baseUrl)
.reduce((obj, value) => Object.assign(obj, { [value.key]: value + '' }), {});
.reduce(
(obj, value) => Object.assign(obj, { [value.key]: value + '' }),
{}
);

assert.property(cookies, 'appSession')
assert.notProperty(cookies, 'appSession.0')
assert.property(cookies, 'appSession');
assert.notProperty(cookies, 'appSession.0');
});

it('should handle unordered chunked cookies', async () => {
Expand Down Expand Up @@ -323,6 +340,33 @@ describe('appSession', () => {
});
});

it('should disregard custom id generation without a custom store', async () => {
server = await createServer(
appSession(
getConfig({
...defaultConfig,
session: {
genid: () => {
throw 'this should not be called';
}, //consider using chai-spies
},
})
)
);
const jar = request.jar();
const res = await request.get('/session', {
baseUrl,
json: true,
jar,
headers: {
cookie: `appSession=${encrypted}`,
},
});

assert.equal(res.statusCode, 200);
assert.equal(res.body.sub, '__test_sub__');
});

it('should use a custom cookie name', async () => {
server = await createServer(
appSession(
Expand Down
3 changes: 3 additions & 0 deletions test/config.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,14 @@ describe('get config', () => {
});

it('should set custom cookie configuration', () => {
const sessionIdGenerator = () => '1235';
const config = getConfig({
...defaultConfig,
secret: ['__test_session_secret_1__', '__test_session_secret_2__'],
session: {
name: '__test_custom_session_name__',
rollingDuration: 1234567890,
genid: sessionIdGenerator,
cookie: {
domain: '__test_custom_domain__',
transient: true,
Expand All @@ -188,6 +190,7 @@ describe('get config', () => {
rollingDuration: 1234567890,
absoluteDuration: 604800,
rolling: true,
genid: sessionIdGenerator,
cookie: {
domain: '__test_custom_domain__',
transient: true,
Expand Down