Skip to content
This repository

96 remove universal cors #97

Merged
merged 2 commits into from about 2 years ago

2 participants

Brian J Brennan Chris McAvoy
Brian J Brennan
Collaborator

We had Access-Control-Allow-Origin: * going through on every request, which is a potential security vulnerability. I've modified it so only whitelisted URLs get the CORS header.

Chris McAvoy
Owner

looks good to me...

Chris McAvoy cmcavoy merged commit 317cdc7 into from March 26, 2012
Chris McAvoy cmcavoy closed this March 26, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
13  app.js
@@ -28,13 +28,14 @@ app.helpers({
28 28
   badges: {},
29 29
   reverse: router.reverse,
30 30
 });
  31
+
31 32
 app.dynamicHelpers({
32 33
   user: function(req, res){
33 34
     return req.user || null;
34 35
   }
35 36
 });
36  
-// Middleware. See `middleware.js` for more information on the custom
37  
-// middleware used.
  37
+
  38
+// Middleware. See `middleware.js`
38 39
 app.use(express.static(path.join(__dirname, "static")));
39 40
 app.use(express.static(path.join(configuration.get('var_dir'), "badges")));
40 41
 app.use(middleware.noFrame({ whitelist: [ '/issuer/frame', '/', '/share/.*' ] }));
@@ -45,13 +46,7 @@ app.use(middleware.logRequests());
45 46
 app.use(middleware.cookieSessions());
46 47
 app.use(middleware.userFromSession());
47 48
 app.use(middleware.csrf({ whitelist: ['/issuer/validator/?'] }));
48  
-
49  
-// Allow everything to be used with CORS.
50  
-// This should probably just be limited to badges
51  
-app.use(function(req, res, next) {
52  
-  res.header("Access-Control-Allow-Origin", "*");
53  
-  next();
54  
-});
  49
+app.use(middleware.cors({ whitelist: ['/_badges.*', '/issuer.*', '/baker'] }));
55 50
 
56 51
 router(app)
57 52
   .get('/baker',                      'baker.baker')
10  middleware.js
@@ -94,6 +94,16 @@ exports.noFrame = function(opts) {
94 94
   };
95 95
 };
96 96
 
  97
+exports.cors = function (options) {
  98
+  var options = options || {}
  99
+  var list = options.whitelist || []
  100
+  if (typeof list === 'string') list = [list];
  101
+  return function(req, res, next){
  102
+    if (!whitelisted(list, req.url)) return next();
  103
+    res.header("Access-Control-Allow-Origin", "*");
  104
+    return next();
  105
+  }
  106
+}
97 107
 
98 108
 // #FIXME: This was pulled from connect/lib/middleware/csrf.js
99 109
 //         The current version of the csrf middleware checks the token on
13  test/conmock-test.js
@@ -58,6 +58,19 @@ vows.describe('Connection mocking').addBatch({
58 58
         mock.headers['oh'].should.equal('hai');
59 59
       },
60 60
     },
  61
+    '#next': {
  62
+      topic: function () {
  63
+        function mware (request, response, next) {
  64
+          response.header('oh', 'hai');
  65
+          next();
  66
+        };
  67
+        conmock(mware, {}, this.callback);
  68
+      },
  69
+      'callback gets called, knows headers': function (err, mock) {
  70
+        mock.headers['oh'].should.equal('hai');
  71
+      }
  72
+    },
  73
+
61 74
     '#contentType' : {
62 75
       'given "json"': {
63 76
         topic: function () {
6  test/conmock.js
@@ -44,7 +44,11 @@ conmock = function (fn, request, callback) {
44 44
       callback(null, this);
45 45
     },
46 46
   };
47  
-  return fn(request, response);
  47
+  function next () {
  48
+    response.fntype = 'next';
  49
+    callback(null, response);
  50
+  }
  51
+  return fn(request, response, next);
48 52
 };
49 53
 
50 54
 module.exports = conmock;
55  test/middleware-test.js
... ...
@@ -0,0 +1,55 @@
  1
+var vows = require('vows');
  2
+var assert = require('assert');
  3
+var should = require('should');
  4
+var middleware = require('../middleware.js');
  5
+var conmock = require('./conmock.js');
  6
+
  7
+vows.describe('middlware tests').addBatch({
  8
+  '#cors': {
  9
+    'no whitelist' : {
  10
+      'topic' : function () {
  11
+        var hdlr = middleware.cors();
  12
+        conmock(hdlr, {}, this.callback);
  13
+      },
  14
+      'does not add header' : function (err, mock) {
  15
+        should.not.exist(mock.headers['Access-Control-Allow-Origin'], 'CORS header present when it should not be.');
  16
+      },
  17
+    },
  18
+    'whitelist is a string, url not on it' : {
  19
+      'topic' : function () {
  20
+        var hdlr = middleware.cors({ whitelist: '/foo' });
  21
+        conmock(hdlr, { url: '/bar' }, this.callback);
  22
+      },
  23
+      'does not add header' : function (err, mock) {
  24
+        should.not.exist(mock.headers['Access-Control-Allow-Origin'], 'CORS header present when it should not be.');
  25
+      },
  26
+    },
  27
+    'whitelist is a string, url matches' : {
  28
+      'topic' : function () {
  29
+        var hdlr = middleware.cors({ whitelist: '/foo' });
  30
+        conmock(hdlr, { url: '/foo' }, this.callback);
  31
+      },
  32
+      'should add CORS header' : function (err, mock) {
  33
+        should.exist(mock.headers['Access-Control-Allow-Origin'], 'CORS header not present when it should be.');
  34
+      },
  35
+    },
  36
+    'whitelist is a list, url matches' : {
  37
+      'topic' : function () {
  38
+        var hdlr = middleware.cors({ whitelist: ['/bar', '/f..'] });
  39
+        conmock(hdlr, { url: '/foo' }, this.callback);
  40
+      },
  41
+      'should add CORS header' : function (err, mock) {
  42
+        should.exist(mock.headers['Access-Control-Allow-Origin'], 'CORS header not present when it should be.');
  43
+      },
  44
+    },
  45
+    'whitelist is a list, url does not match' : {
  46
+      'topic' : function () {
  47
+        var hdlr = middleware.cors({ whitelist: ['/bar', '/f..'] });
  48
+        conmock(hdlr, { url: '/rad' }, this.callback);
  49
+      },
  50
+      'should add CORS header' : function (err, mock) {
  51
+        should.not.exist(mock.headers['Access-Control-Allow-Origin'], 'CORS header present when it should not be.');
  52
+      },
  53
+    },
  54
+  },
  55
+}).export(module);
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.