Skip to content

Commit

Permalink
fix(security): upgrade to @koa/cors 5.0.0 (#20)
Browse files Browse the repository at this point in the history
* fix(security): upgrade to @koa/cors 5.0.0
* chores(workflow): remove Node 16 from the test grid
* chores(package): update dependencies

BREAKING CHANGE: CORS behavior is changed to enforced SOP security, see README
BREAKING CHANGE: Dropping support of Node 16
  • Loading branch information
arnaudbuchholz-sap authored Jan 9, 2024
1 parent 83fa212 commit 656555f
Show file tree
Hide file tree
Showing 6 changed files with 1,140 additions and 32 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:

strategy:
matrix:
node-version: [16.x, 18.x, 20.x]
node-version: [18.x, 20.x]
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

steps:
Expand Down
17 changes: 16 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,27 @@ put these lines in your server.js

## Build-in Middlewares

### Cors Support ([koa-cors](https://github.com/evert0n/koa-cors))
### Cors Support ([@koa/cors](https://www.npmjs.com/package/@koa/cors))

``` javascript
app.addCorsSupportMiddleware();
```

Allowed settings :
``` javascript
app.addCorsSupportMiddleware({
allowOrigin: '*', // `Access-Control-Allow-Origin`, * or a regex to filter allowed origins (for instance /emarsys.(com|net)$/)
allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH', // `Access-Control-Allow-Methods`
});
```

**⚠️ WARNING ⚠️** :
Not specifying an allowed origin made the middleware always return an `Access-Control-Allow-Origin` header with the value of the origin from the request. This behavior completely disables one of the most crucial elements of browsers - the Same Origin Policy (SOP), this could cause a very serious security threat to the users of this middleware.

Since version `2.0.0`, the package is based `@koa/[email protected]` which
[disables this behavior](https://www.npmjs.com/package/@koa/cors/v/5.0.0#breaking-change-between-50-and-40).
It is **highly recommended** to specify a list of allowed origins.

### Method Override ([koa-methodoverwrite](https://github.com/koa-modules/methodoverride))

``` javascript
Expand Down
23 changes: 20 additions & 3 deletions app/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
'use strict';

let fs = require('fs');
let cors = require('kcors');
let cors = require('@koa/cors');
let methodOverride = require('koa-methodoverride');
let bodyparser = require('koa-bodyparser');
let requestId = require('koa-requestid');
Expand All @@ -14,9 +14,26 @@ class App {
this.koaApp = koaApp;
}

addCorsSupportMiddleware() {
addCorsSupportMiddleware({ allowOrigin, allowMethods } = {
allowOrigin: '*',
allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH'
}) {
let origin = '*';
if (allowOrigin instanceof RegExp) {
origin = function(ctx) {
const incoming = ctx.get('Origin');
if (incoming.match(allowOrigin)) {
return incoming;
}
// eslint-disable-next-line security/detect-unsafe-regex
if (incoming.match(/\/localhost(:\d+)?$/)) {
return incoming;
}
};
}
this.addMiddleware(cors({
origin: '*'
origin,
allowMethods
}));
}

Expand Down
146 changes: 146 additions & 0 deletions app/index.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
'use strict';

const Koa = require('koa');
const request = require('supertest');
const App = require('./');
const { expect } = require('chai');

describe('Application building', () => {
describe('CORS', () => {
describe('default allowed origins', () => {
let koa;
let server;

before(() => {
koa = new Koa();
const app = new App(koa);
app.addCorsSupportMiddleware();
koa.use(function(ctx) {
ctx.body = { foo: 'bar' };
});
server = koa.listen();
});

after(() => {
server.close();
});

it('should set `Access-Control-Allow-Origin` to `*` when request Origin header missing', () => {
return request(server)
.get('/')
.expect({ foo: 'bar' })
.expect('access-control-allow-origin', '*')
.expect(200);
});

it('should set `Access-Control-Allow-Origin` to `*`', () => {
return request(server)
.get('/')
.set('Origin', 'http://koajs.com')
.expect('Access-Control-Allow-Origin', '*')
.expect({ foo: 'bar' })
.expect(200);
});

it('should 204 on Preflight Request', () => {
return request(server)
.options('/')
.set('Origin', 'http://koajs.com')
.set('Access-Control-Request-Method', 'PUT')
.expect('Access-Control-Allow-Origin', '*')
.expect('Access-Control-Allow-Methods', 'GET,HEAD,PUT,POST,DELETE,PATCH')
.expect(204);
});

it('should not Preflight Request if request missing Access-Control-Request-Method', () => {
return request(server)
.options('/')
.set('Origin', 'http://koajs.com')
.expect(200);
});

it('should always set `Vary` to Origin', () => {
return request(server)
.get('/')
.set('Origin', 'http://koajs.com')
.expect('Vary', 'Origin')
.expect({ foo: 'bar' })
.expect(200);
});
});

describe('specific settings', () => {
let koa;
let server;

before(() => {
koa = new Koa();
const app = new App(koa);
app.addCorsSupportMiddleware({
allowOrigin: /emarsys\.(com|net)$/,
allowMethods: 'GET,POST'
});
koa.use(function(ctx) {
ctx.body = { foo: 'bar' };
});
server = koa.listen();
});

after(() => {
server.close();
});

it('should not set `Access-Control-Allow-Origin` when request Origin header missing', async () => {
const response = await request(server)
.get('/')
.expect({ foo: 'bar' })
.expect(200);
expect(response.headers['access-control-allow-origin']).to.be.undefined;
});

it('should not set `Access-Control-Allow-Origin` when request Origin does not match', async () => {
const response = await request(server)
.get('/')
.set('Origin', 'http://koajs.com')
.expect({ foo: 'bar' })
.expect(200);
expect(response.headers['access-control-allow-origin']).to.be.undefined;
});

it('should set `Access-Control-Allow-Origin` when request Origin match', () => {
return request(server)
.get('/')
.set('Origin', 'https://anyserver.emarsys.com')
.expect({ foo: 'bar' })
.expect('access-control-allow-origin', 'https://anyserver.emarsys.com')
.expect(200);
});

it('should 204 on Preflight Request', () => {
return request(server)
.options('/')
.set('Origin', 'https://anyserver.emarsys.com')
.set('Access-Control-Request-Method', 'GET,POST')
.expect('Access-Control-Allow-Origin', 'https://anyserver.emarsys.com')
.expect(204);
});

it('should 204 on Preflight Request for local development', () => {
return request(server)
.options('/')
.set('Origin', 'http://localhost:4000')
.set('Access-Control-Request-Method', 'GET,POST')
.expect('Access-Control-Allow-Origin', 'http://localhost:4000')
.expect(204);
});

it('should not Preflight Request if request Origin does not match', async () => {
const response = await request(server)
.options('/')
.set('Origin', 'http://koajs.com')
.expect(200);
expect(response.headers['access-control-allow-origin']).to.be.undefined;
});
});
});
});
Loading

0 comments on commit 656555f

Please sign in to comment.