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

fix: add regenerate method for session security #3499

Merged
merged 1 commit into from
Dec 14, 2023
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
7 changes: 6 additions & 1 deletion packages/session/src/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,15 @@ export interface ISession {
*/
manuallyCommit(): Promise<void>;

/**
* regenerate this session
*/
regenerate(callback?: () => void): void;

/**
* save this session no matter whether it is populated
*/
save(): void;
save(callback?: () => void): void;

/**
* allow to put any value on session object
Expand Down
16 changes: 11 additions & 5 deletions packages/session/src/lib/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ export class ContextSession {
* @api public
*/

async commit() {
async commit({ save = false, regenerate = false } = {}) {
const session = this.session;
const opts = this.opts;
const ctx = this.ctx;
Expand All @@ -237,7 +237,16 @@ export class ContextSession {
return;
}

const reason = this._shouldSaveSession();
if (regenerate) {
await this.remove();
if (this.store) this.externalKey = opts.genid && opts.genid(ctx);
}

// force save session when `session._requireSave` set
const reason =
save || regenerate || session._requireSave
? 'force'
: this._shouldSaveSession();
debug('should save session: %s', reason);
if (!reason) return;

Expand All @@ -253,9 +262,6 @@ export class ContextSession {
const prevHash = this.prevHash;
const session = this.session;

// force save session when `session._requireSave` set
if (session._requireSave) return 'force';

// do nothing if new and not populated
const json = session.toJSON();
if (!prevHash && !Object.keys(json).length) return '';
Expand Down
32 changes: 28 additions & 4 deletions packages/session/src/lib/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,19 @@ export class Session implements ISession {
* @api public
*/

save() {
this._requireSave = true;
save(callback) {
return this.commit({ save: true }, callback);
}

/**
* regenerate this session
*
* @param {Function} callback the optional function to call after regenerating the session
* @api public
*/

regenerate(callback) {
return this.commit({ regenerate: true }, callback);
}

/**
Expand All @@ -122,7 +133,20 @@ export class Session implements ISession {
* @api public
*/

async manuallyCommit() {
await this._sessCtx.commit();
manuallyCommit() {
return this.commit();
}

commit(options?, callback?) {
if (typeof options === 'function') {
callback = options;
options = {};
}
const promise = this._sessCtx.commit(options);
if (callback) {
promise.then(() => callback(), callback);
} else {
return promise;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { Configuration, Controller, Get } from '@midwayjs/core';
import * as koa from '../../../../../web-koa';
import * as assert from 'assert';

@Configuration({
imports: [
koa,
],
importConfigs: [
{
default: {
keys: '12345',
session: {
sameSite: 'none',
}
}
}
]
})
export class AutoConfiguration {
async onReady(container, app) {
const message = 'hi';
app.useMiddleware(async (ctx, next) => {
ctx.session = { message: 'hi' };
await next();
});

app.useMiddleware(async function(ctx, next) {
const sessionKey = ctx.cookies.get('MW_SESS', { signed: false });
if (sessionKey) {
await ctx.session.regenerate();
}
await next();
});

app.useMiddleware(async function(ctx) {
assert(ctx.session.message === message);
ctx.body = '';
});
}
}

@Controller('/')
export class HomeController {

@Get('/get')
async get(ctx) {
return ctx.session;
}
@Get('/set')
async set(ctx) {
ctx.session = ctx.query;
ctx.body = ctx.session;
}
}
23 changes: 23 additions & 0 deletions packages/session/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,4 +158,27 @@ describe('test/index.test.ts', function () {

await close(app);
});

it('should change the session key, but not content', async () => {
const app = await createApp(join(__dirname, 'fixtures/change-session-key'));
const request = createHttpRequest(app);
let koaSession = null;
await request.get('/')
.expect(200)
.expect((res) => {
koaSession = res.headers['set-cookie'][0];
assert(koaSession.match(/MW_SESS=.*?;/));
});

await request.get('/')
.set('Cookie', koaSession)
.expect(200)
.expect((res) => {
const cookies = res.headers['set-cookie'][0];
assert(cookies.match(/MW_SESS=.*?;/));
assert(cookies.replace(/;.*/, '') !== koaSession.replace(/;.*/, ''));
});

await close(app);
});
});
Loading